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)
|
The bias, dark, flat, fringe, etc., should not be read unless they're going to be applied. Note that the fringe subtraction is not necessarily performed if doFringe is True, but it also looks at the filter.
Use a Struct instead of a dict for the output of readDetrendData.
The word "detrend" was inherited from Pan-STARRS and CFHT, but it hasn't historically been part of LSST-speak. This might be a good time to replace it with whatever LSST uses ("calib"?).