DM-13637: Prefilter artifact candidates based on bad pixel mask #194
Conversation
|
|
||
| # Remove artifacts due to defects before they contribute to the epochCountImage | ||
| if self.config.doPrefilterDefects: | ||
| spanSetList = self._prefilterArtifacts(spanSetList, warpDiffExp) |
There was a problem hiding this comment.
I suggest naming the configuration parameter and the method name so that they use the same terminology: "defects" vs "artifacts".
There was a problem hiding this comment.
Sure. Renamed Defects to Artifacts on all three config parameters to match.
| @@ -1746,6 +1766,32 @@ def findArtifacts(self, templateCoadd, tempExpRefList, imageScalerList): | |||
| 'EDGE': edge}) | |||
| return altMasks | |||
|
|
|||
| def _prefilterArtifacts(self, spanSetList, exp): | |||
| """! | |||
| \brief Remove artifact candidates covered by bad mask plane | |||
There was a problem hiding this comment.
I suggest using numpydoc, even if it doesn't match what's already in the file --- but maybe that's naughty.
My reasoning is that it looks better, doxygen isn't going to parse python for much longer (if it still is), and someone will thank you when it comes time to update.
There was a problem hiding this comment.
I promise that future person will be me: DM-13587
|
|
||
| return List of SpanSets with artifacts | ||
| """ | ||
| badPixelMask = afwImage.Mask.getPlaneBitMask(self.config.prefilterDefectsMaskPlanes) |
There was a problem hiding this comment.
I think you should prefer exp.mask.getPlanceBitMask(...).
| return List of SpanSets with artifacts | ||
| """ | ||
| badPixelMask = afwImage.Mask.getPlaneBitMask(self.config.prefilterDefectsMaskPlanes) | ||
| goodArr = numpy.where(exp.mask.array & badPixelMask, False, True) |
There was a problem hiding this comment.
Maybe clearer: numpy.where((exp.mask.array & badPixelMask) == 0))
| for i, span in enumerate(spanSetList): | ||
| y, x = span.clippedTo(bbox).indices() | ||
| yIdxLocal = [y1 - y0 for y1 in y] | ||
| xIdxLocal = [x1 - x0 for x1 in x] |
There was a problem hiding this comment.
Aren't x and y numpy arrays, so you can just do:
yIdxLocal = y - y0
xIdxLocal = x - x0
If they're not numpy arrays, just cast them.
And I think you should spell out xIndexLocal.
There was a problem hiding this comment.
This failed a unit test. Reverting.
y, x = span.clippedTo(bbox).indices()
yIndexLocal = numpy.array(y) - y0
xIndexLocal = numpy.array(x) - x0
> goodRatio = numpy.count_nonzero(goodArr[yIndexLocal, xIndexLocal])/span.getArea()
E TypeError: tuple indices must be integers or slices, not tuple
| y, x = span.clippedTo(bbox).indices() | ||
| yIdxLocal = [y1 - y0 for y1 in y] | ||
| xIdxLocal = [x1 - x0 for x1 in x] | ||
| goodRatio = numpy.count_nonzero(goodArr[yIdxLocal, xIdxLocal])/span.getArea() |
There was a problem hiding this comment.
I would have written this as goodArr[yIndexLocal, xIndexLocal].sum()/span.getArea(), but yours works too, perhaps even clearer.
| @@ -1746,6 +1766,32 @@ def findArtifacts(self, templateCoadd, tempExpRefList, imageScalerList): | |||
| 'EDGE': edge}) | |||
| return altMasks | |||
|
|
|||
| def _prefilterArtifacts(self, spanSetList, exp): | |||
There was a problem hiding this comment.
Suggest dropping the leading underscore. I don't think the method is as private as this indicates.
| @@ -1394,6 +1394,22 @@ class CompareWarpAssembleCoaddConfig(AssembleCoaddConfig): | |||
| dtype=bool, | |||
| default=True, | |||
| ) | |||
| doPrefilterDefects = pexConfig.Field( | |||
| doc="Ignore artifacts that are mostly covered by the bad pixel mask," | |||
There was a problem hiding this comment.
What does it mean to "ignore" an artifact? It's not that the pixels' contributions to the coadd are ignored, right? That's covered by other configuration parameters. But it's the pixels' contributions to the count image. Why is that important?
yalsayyad
force-pushed
the
tickets/DM-13637
branch
3 times, most recently
from
3572a7e
to
9bfd702
Compare
Overlapping defects can eat up the temporal budget and are excluded from coadds by their masks. Remove these artifact candidates from the list before they contribute to the epochCountImage.
Both _filterArtifacts and _prefilterArtifacts are documented like public methods
yalsayyad
force-pushed
the
tickets/DM-13637
branch
from
9bfd702
to
535ac70
Compare
PaulPrice
changed the title
Overlapping defects can eat up the temporal budget and are excluded
from coadds by their masks. Remove these artifact candidates from the
list before they contribute to the epochCountImage.