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

Use Afterburners to clean up aperture correction logic

    Details

    • Type: Story
    • Status: Done
    • Priority: Major
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: meas_base
    • Labels:
      None
    • Templates:
    • Story Points:
      6
    • Sprint:
      DRP X16-3, DRP F16-1, DRP F16-2
    • Team:
      Data Release Production

      Description

      This issue has several components; I'm combining them into a single issue because they need to be done atomically:

      • Rewrite the base_ClassificationExtendedness SingleFramePlugin/ForcedPlugin as an AfterburnerPlugin (and remove the old versions).
      • Move the "applyApCorr" subtask out of SingleFrameMeasurementTask and ForcedMeasurementTask, making it instead a subclass of their parent Tasks.
      • Add afterburner subtask stages to ProcessCcdTask (within DetectAndMeasureTask) and the multiband tasks wherever measurement is currently being run. The afterburner tasks should be run after aperture corrections are measured and/or applied.

      After these changes, throughout the stack, whenever a MeasurementTask is run, we also run ApplyApCorrTask and AfterburnerTask (in that order), while possibly running MeasureApCorrTask immediately after the MeasurementTask.

      This may or may not enable significant cleanups in DetectAndMeasureTask (I haven't looked closely). If so, they should be done on this issue.

      Given all the moving parts, it's important to check that the actual behavior of the pipeline (in the aperture correction and extendedness values) does not change, so it might be useful to start by creating some reference outputs to compare against.

        Issue Links

          Activity

          Hide
          rowen Russell Owen added a comment -

          A possibly simpler approach is to keep the current system but overhaul SingeFrameMeasurement.run as follows:

          • Move most of the implementation into a new method _basicMeasure, but without the code that applies aperture correction
          • Make measureApCorr a subtask of this task
          • Make run look like DetectAndMeasureTask.run except that it applies aperture correction by calling _applyApCorrIfWanted before running the post-ap-corr phase.

          The net result is that sfm would be safe to call with fairly minor code changes, though not completely trivial, since the subtask to measure ap. corr. would move into this task, potentially breaking some existing config overrides.

          However you implement it, I sure hope sfm.run will be simpler and safer to run.

          Show
          rowen Russell Owen added a comment - A possibly simpler approach is to keep the current system but overhaul SingeFrameMeasurement.run as follows: Move most of the implementation into a new method _basicMeasure , but without the code that applies aperture correction Make measureApCorr a subtask of this task Make run look like DetectAndMeasureTask.run except that it applies aperture correction by calling _applyApCorrIfWanted before running the post-ap-corr phase. The net result is that sfm would be safe to call with fairly minor code changes, though not completely trivial, since the subtask to measure ap. corr. would move into this task, potentially breaking some existing config overrides. However you implement it, I sure hope sfm.run will be simpler and safer to run.
          Hide
          jbosch Jim Bosch added a comment -

          I think we're pretty far along in the original proposal already, and I actually like the fact that it gets SingleFrameMeasurementTask out of the business of aperture correction entirely.

          I also think there's a chance this will simplify DetectAndMeasureTask enough that we could just remove it; I'd like to see how it looks first, but I may put together on an RFC on that tomorrow.

          Show
          jbosch Jim Bosch added a comment - I think we're pretty far along in the original proposal already, and I actually like the fact that it gets SingleFrameMeasurementTask out of the business of aperture correction entirely. I also think there's a chance this will simplify DetectAndMeasureTask enough that we could just remove it; I'd like to see how it looks first, but I may put together on an RFC on that tomorrow.
          Hide
          price Paul Price added a comment -

          Reviews completed on github. Major comments:

          • Please factor whitespace and coding style changes into separate commits. Most of them looked gratuitous to me, so I would suggest not bothering about them at all.
          • I'm concerned about the code duplication now present following the removal of the DetectAndMeasureTask. It would be good to have a Task or free function that factors out the common patterns (e.g., detect, deblend, measure, afterburner). Multiple functions could be provided depending on whether aperture corrections are desired.
          Show
          price Paul Price added a comment - Reviews completed on github. Major comments: Please factor whitespace and coding style changes into separate commits. Most of them looked gratuitous to me, so I would suggest not bothering about them at all. I'm concerned about the code duplication now present following the removal of the DetectAndMeasureTask . It would be good to have a Task or free function that factors out the common patterns (e.g., detect, deblend, measure, afterburner). Multiple functions could be provided depending on whether aperture corrections are desired.
          Hide
          nlust Nate Lust added a comment -

          merged to master

          Show
          nlust Nate Lust added a comment - merged to master

            People

            • Assignee:
              nlust Nate Lust
              Reporter:
              jbosch Jim Bosch
              Reviewers:
              Paul Price
              Watchers:
              Jim Bosch, Nate Lust, Paul Price, Russell Owen
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development

                  Agile