DM-25424: Convert Defect to IsrCalib #62
Conversation
This uses the new lsst.ip.isr.Defects calibration class, and follows the crosstalk/linearity path to retain a gen2 interface using the gen3 tasks.
* Fix runner task. * Update docstrings. * Put edgesAsDefects into the correct task config. * Fix !/| typo that was masking entire image. * Add gen2 "inputDimensions" for butler put.
* Correct crosstalk references in pipeline definition. * Fix camera lookup as a 'static calibration.'
I mistakenly dropped an intermediate merge step. The code now matches the previous behavior, and: * measures each input exposure separately * merges "dark" and "flat" defect lists to separate intermediate master using the 70% fraction combination. * merges the intermediate masters with an OR combination. This also fixes an edge case where the final combination was using a '>=' comparison. When the threshold was zero (as for the OR case), this resulted in all pixels being flagged as bad.
pipelines/findDefects.yaml
Outdated
| @@ -0,0 +1,33 @@ | |||
| description: cp_pipe CROSSTALK calibration construction. | |||
python/lsst/cp/pipe/defects.py
Outdated
|
|
||
| class MeasureDefectsTaskConfig(pipeBase.PipelineTaskConfig, | ||
| pipelineConnections=MeasureDefectsConnections): | ||
| """Configuration for measuring linearity from an exposure |
| @@ -119,7 +85,7 @@ class FindDefectsTaskConfig(pexConfig.Config): | |||
| dtype=float, | |||
| doc=("Number of sigma below mean for dark pixel detection. The default value was found to be", | |||
| " appropriate for some LSST sensors in DM-17490."), | |||
| default=5.0, | |||
| default=-5.0, | |||
There was a problem hiding this comment.
Did you change the algorithm, or was this a bug? Before, in darks, we were only finding "bright" pixels (as opposed to flats, where we found both "dark" and "bright" pixels), so that's why this number was positive. The docstring below, in fact, still says "If only a dark is found then only bright
defects will be sought."
There was a problem hiding this comment.
This is an algorithm change. By having these sigma values retain their polarity, the code in findHotAndColdPixels doesn't need external knowledge to determine which way to apply the threshold. The new code simply iterates over a list of sigma values, creates the threshold based on the sign and absolute value of the sigma value, and returns the merged set for that list of values.
I've added a block before the call to findHotAndColdPixels that limits the measurement on DARK frames to only search for bright pixels, while leaving both bright and dark thresholds for all other exposure types. I originally included both for DARK frames, under the assumption that no dark pixels could be found, but this makes the current code closer to the previous version.
| def validate(self): | ||
| super().validate() | ||
| if self.nSigmaBright < 0.0: | ||
| raise ValueError("nSigmaBright must be above 0.0.") | ||
| if self.nSigmaDark > 0.0: | ||
| raise ValueError("nSigmaDark must be below 0.0.") |
There was a problem hiding this comment.
See comment above: before, the thresholds were all positive (number of sigma), and then, down the code, the "dark" and "bright" (i.e., < -nSigma and > nSigma, for nSigma>0) pixels were flagged as defects in the flats, whereas only the "bright" ones (> nSigma) were flagged as defects in the dark.
| nSigma : `list [ `float` ] | ||
| Detection threshold to use. Positive for DETECTED pixels, | ||
| negative for DETECTED_NEGATIVE pixels. |
There was a problem hiding this comment.
Are there any assumptions about the length or contes of this list? i.e., is it expected to be of length=2 (one for darks and one for flats). Above, this list is [self.config.nSigmaBright, self.config.nSigmaDark]. Or, if the user provides some list of floats with length > 2, it's going to iterate over it (regardless of possible redundancies in the defects found, i.e, the set of defects for, say, nSigma > 6 is going to be a subset of the sets of defects for nSigma > 4).
There was a problem hiding this comment.
It probably is only useful in the case that the length is less than or equal to 2. As noted above, I've added a case for DARK inputs frames. I agree that longer lists will be redundant, but this pulls logic out of the findHotAndColdPixels method and puts it in run, so that method only needs to handle the thresholds given. This seemed clearer to me, as from the run method, it's obvious what the find method will do.
| ampImg -= afwMath.makeStatistics(ampImg, afwMath.MEANCLIP, ).getValue() | ||
|
|
||
| mergedSet = None | ||
| for polarity in polarities: | ||
| nSig = self.config.nSigmaBright if polarity else self.config.nSigmaDark | ||
| for sigma in nSigma: |
There was a problem hiding this comment.
See comment above about looping over nSigmas and input nSigma list.
tests/test_defects.py
Outdated
| @@ -587,7 +533,7 @@ def test_pixelCounting(self): | |||
|
|
|||
| # Test the result of _nPixFromDefects() | |||
| # via two different ways of calculating area. | |||
| self.assertEqual(defectArea, task._nPixFromDefects(defects)) | |||
| # self.assertEqual(defectArea, task._nPixFromDefects(defects)) | |||
There was a problem hiding this comment.
If we don't need this anymore (why?), then it should go, no?
There was a problem hiding this comment.
Commented to avoid an error, now fixed and restored.
tests/test_defects.py
Outdated
| def test_edgeMasking(self): | ||
| """Check that the right number of edge pixels are masked by _setEdgeBits()""" | ||
| testImage = self.flatExp.clone() | ||
| mi = testImage.maskedImage | ||
|
|
||
| self.assertEqual(countMaskedPixels(mi, 'EDGE'), 0) | ||
| self.defaultTask._setEdgeBits(mi) | ||
|
|
||
| hEdge = self.defaultConfig.nPixBorderLeftRight | ||
| vEdge = self.defaultConfig.nPixBorderUpDown | ||
| xSize, ySize = mi.getDimensions() | ||
|
|
||
| nEdge = xSize*vEdge*2 + ySize*hEdge*2 - hEdge*vEdge*4 | ||
|
|
||
| self.assertEqual(countMaskedPixels(mi, 'EDGE'), nEdge) |
There was a problem hiding this comment.
Is this test not relevant anymore?
There was a problem hiding this comment.
I think this was a more extreme case of commented to avoid errors during testing leading to deletion. Restored as well.
czwa
force-pushed
the
tickets/DM-25424
branch
from
2d333c4
to
47359c8
Compare
* Fix nested defaultdicts. * Fix crosstalk flux values.
czwa
force-pushed
the
tickets/DM-25424
branch
from
47359c8
to
7faf349
Compare
Update Defect code to use the IsrCalib, and to work in gen3 as well as gen2.