DM-37383: Add package to support DM processing of camera runs #210
Conversation
There was a problem hiding this comment.
First of all, do like the fact that all these isr tasks are getting unique names; I think this will lead to less confusion (as well as enabling single pipelines without clashes). However, I worry that this will break existing workflows (outside of cp_pipe). Does this restructuring need an RFC? (Or at least a wide announcement).
czwa
force-pushed
the
tickets/DM-37383
branch
2 times, most recently
from
102d98c
to
a846613
Compare
python/lsst/cp/pipe/defects.py
Outdated
| @@ -605,7 +605,8 @@ def debugHistogram(self, stepname, ampImage, nSigmaUsed, exp): | |||
| plt.close() | |||
|
|
|||
|
|
|||
| class MeasureDefectsCombinedConnections(MeasureDefectsConnections, | |||
| class MeasureDefectsCombinedConnections(pipeBase.PipelineTaskConnections, | |||
| # MeasureDefectsConnections, | |||
There was a problem hiding this comment.
Is this comment necessary?
python/lsst/cp/pipe/defects.py
Outdated
| class MeasureDefectsCombinedWithFilterConnections(MeasureDefectsCombinedConnections, | ||
| dimensions=("instrument", "detector")): | ||
| class MeasureDefectsCombinedWithFilterConnections(pipeBase.PipelineTaskConnections, | ||
| # MeasureDefectsCombinedConnections, |
There was a problem hiding this comment.
What does this comment mean?
There was a problem hiding this comment.
This comment and the previous one removed, as we're avoiding potential connection inheritance issues.
| # is what MergeDefectsTask expects. If there are multiple, | ||
| # use the one with the most inputs. | ||
| tempList = [self.chooseBest(inputs['inputFlatDefects']), | ||
| self.chooseBest(inputs['inputDarkDefects'])] |
There was a problem hiding this comment.
So the decision is to just use the one with the most inputs, rather than try to merge all of them from the different filters? Have you confirmed that this does something sensible?
There was a problem hiding this comment.
In the {{cp_testing}} processing, all flats are generated prior to these defects, so the task needs to handle the existence of multiple inputs. I didn't want to simply {{OR}} all of the inputs, as the verify ticket for combined defects showed that the dust spots can trigger masking if they're fainter than the threshold. {{OR}}ing these together means that the output defects may have all faint areas in all dust spots in all filters contributing to the masks.
I will admit that choosing the one with the most inputs is completely arbitrary. However, this is somewhat artificial problem; In general processing, the flats would be inspected prior to use, and a single optimal input selected. Only in the {{cp_testing}} case does this decision need to be made automatically.
czwa
force-pushed
the
tickets/DM-37383
branch
from
c7ed724
to
1f24a3f
Compare
Use explicit names for the ISR task calls.