From 320c1157b8ac43dcfef2632660bdd566727d0e8a Mon Sep 17 00:00:00 2001 From: Arun Kannawadi Date: Mon, 16 Sep 2024 15:22:58 -0400 Subject: [PATCH 1/3] Reorder and pad artifact mask handles --- python/lsst/drp/tasks/assemble_coadd.py | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/python/lsst/drp/tasks/assemble_coadd.py b/python/lsst/drp/tasks/assemble_coadd.py index b823f71f..dffd1017 100644 --- a/python/lsst/drp/tasks/assemble_coadd.py +++ b/python/lsst/drp/tasks/assemble_coadd.py @@ -441,19 +441,13 @@ def runQuantum(self, butlerQC, inputRefs, outputRefs): log.warning("doMaskBrightObjects is set to True, but brightObjectMask not loaded") self.processResults(retStruct.coaddExposure, inputData["brightObjectMask"], outputDataId) - if self.config.doWriteArtifactMasks and supplementaryData: # branch to see if running CompareWarp. - dataIds = [ref.dataId for ref in inputs.warpRefList] - psfMatchedDataIds = [ref.dataId for ref in supplementaryData.warpRefList] - - if dataIds != psfMatchedDataIds: - supplementaryData.warpRefList = reorderAndPadList( - supplementaryData.warpRefList, psfMatchedDataIds, dataIds - ) - for warpRef, altMask, outputRef in zip( - inputs.warpRefList, retStruct.altMaskList, outputRefs.artifactMasks - ): - if warpRef is None: - continue + if self.config.doWriteArtifactMasks and isinstance(self, CompareWarpAssembleCoaddTask): + artifactMasksRefList = reorderAndPadList( + outputRefs.artifactMasks, + [ref.dataId for ref in outputRefs.artifactMasks], + [ref.dataId for ref in inputs.warpRefList], + ) + for altMask, outputRef in zip(retStruct.altMaskList, artifactMasksRefList, strict=True): mask = afwImage.Mask(retStruct.coaddExposure.getBBox()) self.applyAltMaskPlanes(mask, altMask) butlerQC.put(mask, outputRef) From d37bb90b9d2e64282c33708a62f4d7ce82fcda19 Mon Sep 17 00:00:00 2001 From: Arun Kannawadi Date: Tue, 17 Sep 2024 09:52:21 -0400 Subject: [PATCH 2/3] Modernize exception in validate --- python/lsst/drp/tasks/assemble_coadd.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/python/lsst/drp/tasks/assemble_coadd.py b/python/lsst/drp/tasks/assemble_coadd.py index dffd1017..399877ff 100644 --- a/python/lsst/drp/tasks/assemble_coadd.py +++ b/python/lsst/drp/tasks/assemble_coadd.py @@ -293,9 +293,11 @@ def setDefaults(self): def validate(self): super().validate() if self.doInterp and self.statistic not in ["MEAN", "MEDIAN", "MEANCLIP", "VARIANCE", "VARIANCECLIP"]: - raise ValueError( - "Must set doInterp=False for statistic=%s, which does not " - "compute and set a non-zero coadd variance estimate." % (self.statistic) + raise pexConfig.FieldValidationError( + self.__class__.doInterp, + self, + f"Must set doInterp=False for statistic={self.statistic}, which does not " + "compute and set a non-zero coadd variance estimate.", ) unstackableStats = ["NOTHING", "ERROR", "ORMASK"] @@ -303,8 +305,10 @@ def validate(self): stackableStats = [ str(k) for k in afwMath.Property.__members__.keys() if str(k) not in unstackableStats ] - raise ValueError( - "statistic %s is not allowed. Please choose one of %s." % (self.statistic, stackableStats) + raise pexConfig.FieldValidationError( + self.__class__.statistic, + self, + f"statistic {self.statistic} is not allowed. Please choose one of {stackableStats}.", ) From afcb1ef3d6bd22342432705e5ec3dad3be2996e8 Mon Sep 17 00:00:00 2001 From: Arun Kannawadi Date: Tue, 17 Sep 2024 10:05:16 -0400 Subject: [PATCH 3/3] Check that doWriteArtifactMask is on only for CompareWarp --- python/lsst/drp/tasks/assemble_coadd.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/python/lsst/drp/tasks/assemble_coadd.py b/python/lsst/drp/tasks/assemble_coadd.py index 399877ff..d5818def 100644 --- a/python/lsst/drp/tasks/assemble_coadd.py +++ b/python/lsst/drp/tasks/assemble_coadd.py @@ -206,7 +206,7 @@ class AssembleCoaddConfig( default=True, ) doWriteArtifactMasks = pexConfig.Field( - doc="Persist artifact masks?", + doc="Persist artifact masks? Should be True for CompareWarp only.", dtype=bool, default=False, ) @@ -311,6 +311,15 @@ def validate(self): f"statistic {self.statistic} is not allowed. Please choose one of {stackableStats}.", ) + # Admittedly, it's odd for a parent class to condition on a child class + # but such is the case until the CompareWarp refactor in DM-38630. + if self.doWriteArtifactMasks and not isinstance(self, CompareWarpAssembleCoaddConfig): + raise pexConfig.FieldValidationError( + self.__class__.doWriteArtifactMasks, + self, + "doWriteArtifactMasks is only valid for CompareWarpAssembleCoaddConfig.", + ) + class AssembleCoaddTask(CoaddBaseTask, pipeBase.PipelineTask): """Assemble a coadded image from a set of warps. @@ -445,7 +454,7 @@ def runQuantum(self, butlerQC, inputRefs, outputRefs): log.warning("doMaskBrightObjects is set to True, but brightObjectMask not loaded") self.processResults(retStruct.coaddExposure, inputData["brightObjectMask"], outputDataId) - if self.config.doWriteArtifactMasks and isinstance(self, CompareWarpAssembleCoaddTask): + if self.config.doWriteArtifactMasks: artifactMasksRefList = reorderAndPadList( outputRefs.artifactMasks, [ref.dataId for ref in outputRefs.artifactMasks],