tickets/DM-13410 #186

yalsayyad · 2018-02-16T21:21:08Z

No description provided.

PaulPrice · 2018-02-16T22:25:34Z

python/lsst/pipe/tasks/assembleCoadd.py

@@ -563,6 +568,18 @@ def assembleMetadata(self, coaddExposure, tempExpRefList, weightList):

        for tempExp, weight in zip(tempExpList, weightList):
            self.inputRecorder.addVisitToCoadd(coaddInputs, tempExp, weight)
+
+        if self.config.doUsePsfMatchedPolygons:


Suggest putting this block in its own method.

PaulPrice · 2018-02-16T22:31:53Z

python/lsst/pipe/tasks/assembleCoadd.py

@@ -1361,6 +1378,8 @@ class CompareWarpAssembleCoaddConfig(AssembleCoaddConfig):
    def setDefaults(self):
        AssembleCoaddConfig.setDefaults(self)
        self.statistic = 'MEAN'
+        self.doUsePsfMatchedPolygons = True
+        self.badMaskPlanes.remove('EDGE')


if "EDGE" in self.badMaskPlanes: self.badMaskPlanes.remove("EDGE")

?

This scares me. EDGE is in there for a reason, at least for HSC --- we get charge pooling around the edges of some CCDs. But maybe warpCompare is good enough at clipping that out now?

Added a comment explaining that we're still removing the EDGE.

PaulPrice · 2018-02-16T22:39:51Z

python/lsst/pipe/tasks/assembleCoadd.py

@@ -1640,16 +1663,21 @@ def findArtifacts(self, templateCoadd, tempExpRefList, imageScalerList):
                nans = numpy.where(numpy.isnan(warpDiffExp.maskedImage.image.array), 1, 0)
                nansMask = afwImage.makeMaskFromArray(nans.astype(afwImage.MaskPixel))
                nansMask.setXY0(warpDiffExp.getXY0())
+                edgeMask = warpDiffExp.mask
+                edgeMask &= edgeMask.getPlaneBitMask("EDGE")


This line modifies the mask. The following would have the same result as what you're doing except without modifying the mask (which seems desirable):

spanSetEdgeMask = afwGeom.SpanSet.fromMask(edgeMask, edgeMask.getPlaneBitMask("EDGE")).split()

See https://github.com/lsst/afw/blob/master/include/lsst/afw/geom/SpanSet.h#L704

PaulPrice · 2018-02-16T22:41:24Z

python/lsst/pipe/tasks/assembleCoadd.py

@@ -1640,16 +1663,21 @@ def findArtifacts(self, templateCoadd, tempExpRefList, imageScalerList):
                nans = numpy.where(numpy.isnan(warpDiffExp.maskedImage.image.array), 1, 0)
                nansMask = afwImage.makeMaskFromArray(nans.astype(afwImage.MaskPixel))
                nansMask.setXY0(warpDiffExp.getXY0())
+                edgeMask = warpDiffExp.mask
+                edgeMask &= edgeMask.getPlaneBitMask("EDGE")
+                spanSetEdgeMask = afwGeom.SpanSet.fromMask(edgeMask).split()
            else:
                # If the directWarp has <1% coverage, the psfMatchedWarp can have 0% and not exist


Just outta curiosity, where is the 1% value set?

Was an just an observation. The unit test has a good example of the PSF-matching kernel eating away the tiny corner of an image. Depends on the size of the image and the matching kernel size.

PaulPrice · 2018-02-16T22:45:09Z

python/lsst/pipe/tasks/coaddBase.py

@@ -77,6 +77,11 @@ class CoaddBaseConfig(pexConfig.Config):
        doc="Apply meas_mosaic ubercal results to input calexps?",
        default=False
    )
+    matchingKernelSize = pexConfig.Field(
+        dtype=int,
+        doc="Size in pixels of matching kernel. Must be odd.",


If it has to be odd, then why not make this a half-size so that no matter what value the user chooses, we can always set fullMatchingKernelSize = 2*halfMatchingKernelSize + 1. Otherwise, you should add a validation lambda.

Went with lambda. Thanks for the suggestion.

PaulPrice · 2018-02-16T23:10:29Z

python/lsst/pipe/tasks/assembleCoadd.py

@@ -1607,8 +1618,30 @@ def assemble(self, skyInfo, tempExpRefList, imageScalerList, weightList,
        badMaskPlanes.append("CLIPPED")
        badPixelMask = afwImage.Mask.getPlaneBitMask(badMaskPlanes)

-        return AssembleCoaddTask.assemble(self, skyInfo, tempExpRefList, imageScalerList, weightList,
-                                          spanSetMaskList, mask=badPixelMask)
+        retStruct = AssembleCoaddTask.assemble(self, skyInfo, tempExpRefList, imageScalerList, weightList,


Suggest using the name result --- less Hungarian.

PaulPrice · 2018-02-16T23:16:24Z

python/lsst/pipe/tasks/assembleCoadd.py

+
+    def applyAltEdgeMask(self, mask, altMaskList):
+        """!
+        \brief Propagate alt EDGE mask to SENSOR_EDGE AND INEXACT_PSF planes


Guessing you're only using doxygen to keep things consistent, right? Otherwise, numpydoc.

I'd rather keep the whole file consistent. I just filed a ticket DM-13587

PaulPrice · 2018-02-16T23:17:44Z

python/lsst/pipe/tasks/assembleCoadd.py

+        \brief Propagate alt EDGE mask to SENSOR_EDGE AND INEXACT_PSF planes
+
+        @param mask: original mask
+        @param altMaskList:  List of Dictionaries containing spanSet lists.


Dictionaries --> dict

PaulPrice · 2018-02-16T23:18:29Z

python/lsst/pipe/tasks/assembleCoadd.py

+        """
+        maskSensorEdgeBit = mask.addMaskPlane("SENSOR_EDGE")
+        maskInexactPsfBit = mask.addMaskPlane("INEXACT_PSF")
+        maskValue = 2**maskSensorEdgeBit | 2**maskInexactPsfBit


Cleaner: maskValue = mask.getPlaneBitMask(["SENSOR_EDGE", "INEXACT_PSF"]).

Changed it, but how do we know that the coadd mask has a SENSOR_EDGE plane and INEXACT_PSF plane already.

PaulPrice · 2018-02-16T23:20:52Z

python/lsst/pipe/tasks/assembleCoadd.py

+        maskInexactPsfBit = mask.addMaskPlane("INEXACT_PSF")
+        maskValue = 2**maskSensorEdgeBit | 2**maskInexactPsfBit
+        for visitMask in altMaskList:
+            for spanSet in visitMask['EDGE']:


How do you know visitMask has an EDGE entry?

WarpCompare writes 3 hardcoded alternative masks: CLIPPED, NO_DATA, EDGE.
https://github.com/lsst/pipe_tasks/compare/tickets/DM-13410#diff-f9cf63fcf2aed29eec448bae6d243b5fR1729 I put an check in here, but hope that someone doesn't remove EDGE later.

CompareWarp uses temporal information from PSF-matched warps that have been shrunk by half the matching kernel. Only pixels in the direct Warp that have coverage in the PSF-matched warps are coadded. The CoaddPsf should therefore use these smaller polygons for computing the visits that have contributed to the PSF. This change also creates config in coaddBase 'matchingKernelSize' that controls the matchingKernel in PSF-matching in makeCoaddTempExp and the number of pixels to shrink the valid polygons in CompareWarpAssembleCoadd.

* Remove directWarp's original EDGE plane; the true edge had moved inwards * Allow directWarp's updated EDGE pixels (from PSF-matched EDGE) into the coadd, but mark as SENSOR_EDGE. * Clear mask planes that would have been rejected outside the new valid polygon, and reject as NO_DATA instead, so that they do not propgate to INEXACT_PSF. This behavior may be turned off by setting doUsePsfMatchedPolygons=False.

yalsayyad force-pushed the tickets/DM-13410 branch from bdc1ced to 4df578c Compare February 16, 2018 21:27

PaulPrice approved these changes Feb 16, 2018

View reviewed changes

yalsayyad force-pushed the tickets/DM-13410 branch from 4df578c to ac5fb41 Compare February 20, 2018 07:14

yalsayyad added 2 commits February 20, 2018 01:16

yalsayyad force-pushed the tickets/DM-13410 branch from ac5fb41 to 8c9a97e Compare February 20, 2018 07:16

yalsayyad merged commit 8c9a97e into master Feb 20, 2018

ktlim deleted the tickets/DM-13410 branch August 25, 2018 06:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tickets/DM-13410 #186

tickets/DM-13410 #186

yalsayyad commented Feb 16, 2018

PaulPrice Feb 16, 2018

PaulPrice Feb 16, 2018

PaulPrice Feb 16, 2018

yalsayyad Feb 20, 2018

PaulPrice Feb 16, 2018

PaulPrice Feb 16, 2018

yalsayyad Feb 20, 2018

PaulPrice Feb 16, 2018

yalsayyad Feb 20, 2018

PaulPrice Feb 16, 2018

PaulPrice Feb 16, 2018

yalsayyad Feb 20, 2018

PaulPrice Feb 16, 2018

PaulPrice Feb 16, 2018

yalsayyad Feb 20, 2018

PaulPrice Feb 16, 2018

yalsayyad Feb 20, 2018

tickets/DM-13410 #186

tickets/DM-13410 #186

Conversation

yalsayyad commented Feb 16, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment