Uploaded image for project: 'Request For Comments'
  1. Request For Comments
  2. RFC-26

API Change for IsrTask (Backwards-compatible)

    Details

    • Type: RFC
    • Status: Retired
    • Resolution: Done
    • Component/s: DM
    • Labels:
      None
    • Location:
      Issue comments

      Description

      During the S14 efforts to document the important PipeTasks, it was pointed out that the only way to use some of the low level tasks is to supply a ButlerDataRef. This discourages reuse and requires code examples/tests to create a fake ButlerDataRef. This RFC proposes to add a non-blob interface to IsrTask specficially (epic: DM-1113). Currently, the commandlineTask processCcd constructs a DataRef from the commandline arguments. processCcd.run() calls isrTask.run(dataRef) which retrieves the calibration data and removes the instrument signature.

      Proposal: Keep the responsibility of retrieving the calibration data in IsrTask (and more generally always give lower-level tasks (e.g. FringeTask) responsibility of reading necessary data from the butler). Split run(sensorRef) into 3 methods:

      • run(sensorRef) which wraps a two new methods
      • readDetrendData(dataRef)} and
      • doIsr(ccdExposure, biasExposure, darkExposure, flatExposure, defects, fringes).

      See ip_isr branch /u/yusra/DM-1299 for a prototype or pseudocode example below.

      A benefit to having a separate method to read the data is the hierarchical symmetry with lower-level subtasks of IsrTask such as FringeTask. This proposal will also require an analogous refactoring of FringeTask.

      A complication is that IsrTask is the most camera-specific task in the stack and requires a lot of input. readDetrendData and doIsr (if not run too) will likely need to be overridden in obs_*. In obs_lsstSim for example, run could also be responsible for combining snaps.

      This change should not break any client code, but will require an analogous refactoring of the obs_* IsrTasks, and a clean up of examples and unit tests that create fake dataRefs. Also note the choice of method names run/doIsr instead of runDataRef/run. Rational was that run will still be the primary method called and this maintains backwards compatibility.

      Pseudocode example

      class IsrTask:
          ...
          def readDetrendData(self, sensorRef):
              biasExposure = self.getDetrend(sensorRef, "bias")
              ...
              if self.config.doFringe:
                  fringes = self.fringe.readFringes(sensorRef, assembler=self.assembleCcd)
              ...
              return dict of calibration products {biasExposure, ... fringes}
       
          def doIsr(self, ccdExposure, biasExposure=None, darkExposure=None,  flatExposure=None,
                    defects=None, fringes=None):
              #do processing including subtasks, e.g.
              if self.config.doBias:
                  self.biasCorrection(exp, biasExp)
              ...
              if self.config.doFringe:
                  self.fringe.removeFringe(ccdExposure, fringeData, assembler=None)
              ...
       
          def run(self, sensorRef):
              ccdExposure = sensorRef.get('raw')
              detrendData = self.readDetrendData(sensorRef)
              ccdExposure = self.doIsr(ccdExposure, **detrendData).exposure
              ...
       
      class FringeTask:
          def readFringes(self, dataRef, assembler=None):
              ...
          def removeFringe(self, exposure, fringes, assembler=None):
              ...
          def run(self, exposure, dataRef, assembler=None):
              ...
              fringes = self.readFringes(dataRef, assembler=assembler)
              self.removeFringe(exposure, fringes, assembler=assembler)
      

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                yusra Yusra AlSayyad
                Reporter:
                yusra Yusra AlSayyad
                Watchers:
                Hsin-Fang Chiang, John Swinbank, Paul Price, Robert Lupton, Russell Owen, Tim Jenness, Xiuqin Wu [X] (Inactive), Yusra AlSayyad
              • Votes:
                0 Vote for this issue
                Watchers:
                8 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Planned End:

                  Summary Panel