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

API Change for IsrTask (Backwards-compatible)



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


      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)


          Issue Links



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


                Planned End:


                  No builds found.