From 70ceaba01301544c2f1cbc4ce32415f1e12e7499 Mon Sep 17 00:00:00 2001 From: Jim Bosch Date: Thu, 17 Oct 2024 15:16:11 -0400 Subject: [PATCH 1/4] Make ReprocessVisitImageTask compatible with older outputs. Defaults are still set to use newer CalibrateImageTask outputs. --- .../lsst/drp/tasks/reprocess_visit_image.py | 35 ++++++++++++------- 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/python/lsst/drp/tasks/reprocess_visit_image.py b/python/lsst/drp/tasks/reprocess_visit_image.py index 459967c1..9620b020 100644 --- a/python/lsst/drp/tasks/reprocess_visit_image.py +++ b/python/lsst/drp/tasks/reprocess_visit_image.py @@ -112,10 +112,10 @@ class ReprocessVisitImageConnections( ) def __init__(self, *, config=None): - super().__init__(config=config) - - if config.do_use_sky_corr is False: - self.inputs.remove("background_2") + if not config.do_use_sky_corr: + del self.background_2 + if not config.remove_initial_photo_calib: + del self.initial_photo_calib class ReprocessVisitImageConfig( @@ -129,6 +129,11 @@ class ReprocessVisitImageConfig( default=True, doc="Include the skyCorr input for background subtraction?", ) + remove_initial_photo_calib = pexConfig.Field( + dtype=bool, + default=True, + doc="Remove an already-applied photometric calibration from the backgrounds?", + ) snap_combine = pexConfig.ConfigurableField( target=snapCombine.SnapCombineTask, doc="Task to combine two snaps to make one exposure.", @@ -314,7 +319,10 @@ def runQuantum(self, butlerQC, inputRefs, outputRefs): exposures = inputs.pop("exposures") visit_summary = inputs.pop("visit_summary") calib_sources = inputs.pop("calib_sources") - initial_photo_calib = inputs.pop("initial_photo_calib") + if self.config.remove_initial_photo_calib: + initial_photo_calib = inputs.pop("initial_photo_calib") + else: + initial_photo_calib = None background_1 = inputs.pop("background_1") if self.config.do_use_sky_corr: background_2 = inputs.pop("background_2") @@ -399,9 +407,10 @@ def run( Modified in-place during processing if only one is passed. If two exposures are passed, treat them as snaps and combine before doing further processing. - initial_photo_calib : `lsst.afw.image.PhotoCalib` + initial_photo_calib : `lsst.afw.image.PhotoCalib` or `None` Photometric calibration that was applied to exposure during the - measurement of the background. + measurement of the background. Should be `None` if and only if + ``config.remove_initial_photo_calib` is false. psf : `lsst.afw.detection.Psf` PSF model for this exposure. background : `lsst.afw.math.BackgroundList` @@ -448,12 +457,14 @@ def run( result.exposure = self.snap_combine.run(exposures).exposure - # Calibrate the image, so it's on the same units as the background. - result.exposure.maskedImage = initial_photo_calib.calibrateImage(result.exposure.maskedImage) + if self.config.remove_initial_photo_calib: + # Calibrate the image, so it's on the same units as the background. + result.exposure.maskedImage = initial_photo_calib.calibrateImage(result.exposure.maskedImage) result.exposure.maskedImage -= background.getImage() - # Uncalibrate so that we do the measurements in instFlux, because - # we don't have a way to identify measurements as being in nJy. - result.exposure.maskedImage /= initial_photo_calib.getCalibrationMean() + if self.config.remove_initial_photo_calib: + # Uncalibrate so that we do the measurements in instFlux, because + # we don't have a way to identify measurements as being in nJy. + result.exposure.maskedImage /= initial_photo_calib.getCalibrationMean() result.exposure.setPsf(psf) result.exposure.setApCorrMap(ap_corr) From e6637cb5a19bdb348ee553fcd225ab0f74e46f69 Mon Sep 17 00:00:00 2001 From: Jim Bosch Date: Fri, 18 Oct 2024 10:19:36 -0400 Subject: [PATCH 2/4] Update TODO entry on streak mask planes. --- python/lsst/drp/tasks/reprocess_visit_image.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/lsst/drp/tasks/reprocess_visit_image.py b/python/lsst/drp/tasks/reprocess_visit_image.py index 9620b020..aa95ad7d 100644 --- a/python/lsst/drp/tasks/reprocess_visit_image.py +++ b/python/lsst/drp/tasks/reprocess_visit_image.py @@ -77,7 +77,7 @@ class ReprocessVisitImageConnections( storageClass="ArrowAstropy", dimensions=["instrument", "visit"], ) - # TODO DM-45980: pull in the STREAK mask from the diffim. + # TODO DM-46947: pull in the STREAK mask from CompareWarp. # outputs sources_schema = connectionTypes.InitOutput( From d35e7be2e083761495c0b8b53274c762835ad47c Mon Sep 17 00:00:00 2001 From: Jim Bosch Date: Fri, 18 Oct 2024 10:56:30 -0400 Subject: [PATCH 3/4] Move ReprocessVisitImage pixel calibration to the end. This puts the output background and summary stats measurements in ADU, making them more consistent with other backgrounds and summary stats produced earlier in the pipeline, but inconsistent with the units of the "pvi" output image. It also makes the LocalWcs and LocalPhotoCalib plugin outputs reflect the transformation from ADU to nJy; I'm pretty sure they were previously just identities, which seems like it was probably a bug, unless downstream transform tasks had been configured to read "flux" fields rather than "instFlux" fields. --- .../lsst/drp/tasks/reprocess_visit_image.py | 28 +++++++++++-------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/python/lsst/drp/tasks/reprocess_visit_image.py b/python/lsst/drp/tasks/reprocess_visit_image.py index aa95ad7d..540ac13d 100644 --- a/python/lsst/drp/tasks/reprocess_visit_image.py +++ b/python/lsst/drp/tasks/reprocess_visit_image.py @@ -105,7 +105,12 @@ class ReprocessVisitImageConnections( dimensions=["instrument", "visit", "detector"], ) background = connectionTypes.Output( - doc="Total background model including new detections in this task.", + doc=( + "Total background model including new detections in this task. " + "Note that the background model has units of ADU, while the corresponding " + "image has units of nJy - the image must be 'uncalibrated' before the background " + "can be restored." + ), name="pvi_background", dimensions=("instrument", "visit", "detector"), storageClass="Background", @@ -285,6 +290,7 @@ def __init__(self, sources_schema=None, **kwargs): ) self.psf_fields = ("calib_psf_candidate", "calib_psf_used", "calib_psf_reserved") + # TODO (DM-46971): # These fields are only here to satisfy the SDM schema, and will # be removed from there as they are misleading (because we don't # propagate this information from gbdes/fgcmcal). @@ -469,20 +475,23 @@ def run( result.exposure.setPsf(psf) result.exposure.setApCorrMap(ap_corr) result.exposure.setWcs(wcs) - # Note: we don't set photoCalib here, because we have to use it below - # to calibrate the image and catalog, and thus the image will have a - # photoCalib of exactly 1. + result.exposure.setPhotoCalib(photo_calib) result.sources_footprints = self._find_sources( result.exposure, background, calib_sources, id_generator ) result.background = background - self._apply_photo_calib(result.exposure, result.sources_footprints, photo_calib) - + # TODO (DM-46971): + # Now that we're running them before we apply the PhotoCalib to the + # image pixels, there's no need for post_calibrations to exist as + # a separate measurement instance from the main one (which is invoked + # in _find_sources), but it's better to save removal (which may need + # to involve a deprecation) until after we've got everything running. self.post_calculations.run(result.sources_footprints, result.exposure) result.exposure.info.setSummaryStats( self.compute_summary_stats.run(result.exposure, result.sources_footprints, background) ) + self._apply_photo_calib(result.exposure, result.sources_footprints, photo_calib) result.sources = result.sources_footprints.asAstropy() return result @@ -584,12 +593,7 @@ def _apply_photo_calib(self, exposure, sources_footprints, photo_calib): supplied PhotoCalib. """ sources_footprints = photo_calib.calibrateCatalog(sources_footprints) - # This is temporary, until we can do the measurements on the - # calibrated image; for now, we have to calibrate to apply the - # background, and then undo it, which is most simply done by taking - # out the mean and redoing it here. - exposure.maskedImage *= photo_calib.getCalibrationMean() - # exposure.maskedImage = photo_calib.calibrateImage(exposure.maskedImage) # noqa + exposure.maskedImage = photo_calib.calibrateImage(exposure.maskedImage) identity = afwImage.PhotoCalib(1.0, photo_calib.getCalibrationErr(), bbox=exposure.getBBox()) exposure.setPhotoCalib(identity) return sources_footprints From f69187c32196fc2c32055f0d9e2cd93651c321e6 Mon Sep 17 00:00:00 2001 From: Yusra AlSayyad Date: Tue, 15 Oct 2024 20:05:44 -0700 Subject: [PATCH 4/4] Set reprocessVisitImage default to not use skyCorr --- python/lsst/drp/tasks/reprocess_visit_image.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/lsst/drp/tasks/reprocess_visit_image.py b/python/lsst/drp/tasks/reprocess_visit_image.py index 540ac13d..90061485 100644 --- a/python/lsst/drp/tasks/reprocess_visit_image.py +++ b/python/lsst/drp/tasks/reprocess_visit_image.py @@ -131,7 +131,7 @@ class ReprocessVisitImageConfig( do_use_sky_corr = pexConfig.Field( dtype=bool, - default=True, + default=False, doc="Include the skyCorr input for background subtraction?", ) remove_initial_photo_calib = pexConfig.Field(