Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-3182

Aperture correction not applied for some measurements

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: meas_base, pipe_tasks
    • Labels:
      None

      Description

      Aperture correction needs to be applied every time a measurement is run after it is first measured in CalibrateTask. As of DM-436 aperture correction is only being applied in CalibrateTask, which for example means the information is overwritten during the final measurement of ProcessImageTask.run.

      This is probably best done by adding code to apply aperture correction to BaseMeasurementTask, so it is inherited by SingleFrameMeasurementTask and ForcedMeasurementTask.

        Attachments

          Issue Links

            Activity

            Hide
            rowen Russell Owen added a comment -

            Jim: I made the changes you suggested via email. I feel they worked out pretty well, but one thing to check is whether you are comfortable having the measurement tasks automatically pull apCorrMap out of the exposure to apply aperture corrections.

            Most changes are in meas_base, but a few are in pipe_tasks, both on branch tickets/DM-3182. I also updated obs_sdss in DM-3114, but that's a separate review.

            Show
            rowen Russell Owen added a comment - Jim: I made the changes you suggested via email. I feel they worked out pretty well, but one thing to check is whether you are comfortable having the measurement tasks automatically pull apCorrMap out of the exposure to apply aperture corrections. Most changes are in meas_base, but a few are in pipe_tasks, both on branch tickets/ DM-3182 . I also updated obs_sdss in DM-3114 , but that's a separate review.
            Hide
            jbosch Jim Bosch added a comment -

            Some comments in the meas_base GitHub PR. Everything in pipe_base looks fine.

            Show
            jbosch Jim Bosch added a comment - Some comments in the meas_base GitHub PR. Everything in pipe_base looks fine.
            Hide
            rowen Russell Owen added a comment - - edited

            In response to the review:

            • I made baseMeasurement.py more analogous to sfm.py and forcedMeasurement.py by moving in the missing base classes. I then split the remaining code in base.py into pluginRegistry.py and apCorrRegistry.py (new modules). I feel this was a definite improvement for readability and organization.
            • I changed the measurement method to raise an exception instead of log a warning if aperture correction wanted but apCorrMap not available in the exposure. That has proven to be a very disruptive change because many tasks, unit tests and examples call measurement (in many packages) but very few of them provide aperture correction data. I strongly suspect there is a better way to handle this and I plan to solicit more feedback before merting this.
            • I got rid of the new method that builds subtasks in BaseMeasurementTask, since it did not add enough functionality to be worth the code.

            I then took a cleanup pass on all the python files, primarily to make them pass the pyflakes linter. This involved removing unused commits, eliminating use of "from foo import *" (except in _init) and defining __all_ in modules where it was missing.

            Show
            rowen Russell Owen added a comment - - edited In response to the review: I made baseMeasurement.py more analogous to sfm.py and forcedMeasurement.py by moving in the missing base classes. I then split the remaining code in base.py into pluginRegistry.py and apCorrRegistry.py (new modules). I feel this was a definite improvement for readability and organization. I changed the measurement method to raise an exception instead of log a warning if aperture correction wanted but apCorrMap not available in the exposure. That has proven to be a very disruptive change because many tasks, unit tests and examples call measurement (in many packages) but very few of them provide aperture correction data. I strongly suspect there is a better way to handle this and I plan to solicit more feedback before merting this. I got rid of the new method that builds subtasks in BaseMeasurementTask, since it did not add enough functionality to be worth the code. I then took a cleanup pass on all the python files, primarily to make them pass the pyflakes linter. This involved removing unused commits, eliminating use of "from foo import *" (except in _ init ) and defining __all _ in modules where it was missing.
            Hide
            rowen Russell Owen added a comment -

            Significant changes committed to meas_base and pipe_tasks. Minor changes committed to , meas_algorithms and ip_diffim.

            Minor changes are also needed in obs_sdss to enable applying aperture correction in SdssCalibrateTask, but that will be done as part of DM-3114.

            Show
            rowen Russell Owen added a comment - Significant changes committed to meas_base and pipe_tasks. Minor changes committed to , meas_algorithms and ip_diffim. Minor changes are also needed in obs_sdss to enable applying aperture correction in SdssCalibrateTask, but that will be done as part of DM-3114 .

              People

              Assignee:
              rowen Russell Owen
              Reporter:
              rowen Russell Owen
              Reviewers:
              Jim Bosch
              Watchers:
              Jim Bosch, Russell Owen
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  CI Builds

                  No builds found.