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

Use Afterburners to clean up aperture correction logic

    XMLWordPrintable

Details

    • Story
    • Status: Done
    • Resolution: Done
    • None
    • meas_base
    • None
    • 6
    • DRP X16-3, DRP F16-1, DRP F16-2
    • 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.

      Attachments

        Issue Links

          Activity

            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.

            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.
            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.

            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.
            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.
            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.
            nlust Nate Lust added a comment -

            merged to master

            nlust Nate Lust added a comment - merged to master

            People

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

              Dates

                Created:
                Updated:
                Resolved:

                Jenkins

                  No builds found.