From 83314b1b4251ee6399645105da3e7d1f35dcf9ce Mon Sep 17 00:00:00 2001 From: Jim Bosch Date: Tue, 17 Dec 2024 12:34:16 -0500 Subject: [PATCH 1/3] Restore forced-astrometry-failure tests. This makes one detector fail to fit astrometry (by making a treshold unrealistically small), so we can test what happens downstream. An extremely similar test was previously removed when we switched to CalibrateImageTask, since that changed how the failure looked to downstream tasks and (at the time) we hadn't yet updated them to behave correctly. --- bin/pipeline.sh | 5 ++- python/lsst/ci/hsc/gen3/data.py | 4 +-- tests/test_astrometryFail.py | 5 --- tests/test_validate_outputs.py | 60 +++++++++++++++++++++------------ 4 files changed, 41 insertions(+), 33 deletions(-) diff --git a/bin/pipeline.sh b/bin/pipeline.sh index 1a92c8d..b5971e8 100755 --- a/bin/pipeline.sh +++ b/bin/pipeline.sh @@ -53,20 +53,19 @@ HIPS_QGRAPH_FILE=$(mktemp)_hips.qgraph trap 'rm -f $QGRAPH_FILE $INJECTION_QGRAPH_FILE $RESOURCE_USAGE_QGRAPH_FILE \ $HIPS_QGRAPH_FILE' EXIT -# DM-46272: Change maxMeanDistanceArcsec to something smaller, so that it fails; -# even better, move those settings into DRP-ci_hsc.yaml! pipetask --long-log --log-level="$loglevel" qgraph \ -d "skymap='discrete/ci_hsc' AND tract=0 AND patch=69" \ -b "$repo"/butler.yaml \ --input "$INPUTCOLL" --output "$COLLECTION" \ -p "$DRP_PIPE_DIR/pipelines/HSC/DRP-ci_hsc.yaml" \ - -c calibrateImage:astrometry.maxMeanDistanceArcsec=0.20 \ + -c calibrateImage:astrometry.maxMeanDistanceArcsec=0.020 \ -c makeDirectWarp:select.maxPsfTraceRadiusDelta=0.2 \ --save-qgraph "$QGRAPH_FILE" pipetask --long-log --log-level="$loglevel" run \ -j "$jobs" -b "$repo"/butler.yaml \ --input "$INPUTCOLL" --output "$COLLECTION" \ + --no-raise-on-partial-outputs \ --register-dataset-types $mock \ --qgraph "$QGRAPH_FILE" diff --git a/python/lsst/ci/hsc/gen3/data.py b/python/lsst/ci/hsc/gen3/data.py index 2e8b01f..f1b326c 100644 --- a/python/lsst/ci/hsc/gen3/data.py +++ b/python/lsst/ci/hsc/gen3/data.py @@ -58,10 +58,8 @@ # set. This list is sensitive to the astrometry algorithms and dataset # under consideration, so may require updating if either of those change # in the context of this repository. -# DM-46272: not forcing these failures until we can handle partial outputs; -# uncomment this line as that ticket is sorted out. ASTROMETRY_FAILURE_DATA_IDS = [ - # {'visit': 903344, 'detector': 0, 'physical_filter': 'HSC-R'}, + {'visit': 903344, 'detector': 0, 'physical_filter': 'HSC-R'}, ] # The following lists the dataIds that fail the PSF Model robustness check # with the config override makeWarp.select.maxPsfTraceRadiusDelta=0.2 set. diff --git a/tests/test_astrometryFail.py b/tests/test_astrometryFail.py index 54f7d0b..e8f6204 100644 --- a/tests/test_astrometryFail.py +++ b/tests/test_astrometryFail.py @@ -29,8 +29,6 @@ from lsst.utils import getPackageDir -# DM-46272: not forcing these failures until we can handle partial outputs; -# remove the expectedFailures as that ticket is sorted out. class TestAstrometryFails(lsst.utils.tests.TestCase): """Tests the outputs of the forced astrometry failures. """ @@ -47,7 +45,6 @@ def setUp(self): universe=self.butler.dimensions, ) - @unittest.expectedFailure def testWcsAndPhotoCalibIsNoneForFailedAstrom(self): """Test the WCS and photoCalib objects attached to failed WCS exposure. @@ -64,7 +61,6 @@ def testWcsAndPhotoCalibIsNoneForFailedAstrom(self): calexpPhotoCalib = self.butler.get("calexp.photoCalib", self.calexpMinimalDataId) self.assertTrue(calexpPhotoCalib is None) - @unittest.expectedFailure def testSrcCoordsAreNanForFailedAstrom(self): """Test coord values in all source catalogs. @@ -103,7 +99,6 @@ def testCentroidsAreNotNanForFailedAstrom(self): self.assertFalse(np.all(np.isnan(sourceCat["x"]))) self.assertFalse(np.all(np.isnan(sourceCat["y"]))) - @unittest.expectedFailure def testVisitCoordsAreNanForFailedAstrom(self): """Test coord and astrom values for visitTable and visitSummary. diff --git a/tests/test_validate_outputs.py b/tests/test_validate_outputs.py index c24254b..2a2f6c6 100644 --- a/tests/test_validate_outputs.py +++ b/tests/test_validate_outputs.py @@ -390,24 +390,41 @@ def test_object_tables(self): self._num_tracts ) + def test_reprocess_visit_image(self): + """Test the existence of ReprocessVisitImageTask's outputs.""" + # While the external WCS files might someday let us recover from the + # forced astrometry failures at this stage, at present that failure + # prevents isolated star association for generating matches for this + # detector, and that prevents the second round of PSF determination and + # aperture correction from working. This manifests as an + # UpstreamFailureNoWorkFound, and hence we expect metadata outputs but + # not source catalogs or PVIs for the forced-failure data IDs. + self.check_pipetasks(["reprocessVisitImage"], len(self._raws), len(self._raws)) + self.check_sources( + ["sources_footprints_detector"], + len(self._raws - self._forced_astrom_failures), + self._min_sources, + additional_checks=[self.check_aperture_corrections] + ) + self.check_datasets(["sources_schema"], 1) + self.check_datasets(["pvi", "pvi_background"], len(self._raws - self._forced_astrom_failures)) + def test_forced_phot_ccd(self): """Test existence of forced photometry tables (sources).""" + # Lack of PVI (from reprocessVisitImage) leads to NoWorkFound for the + # forced astrometry failure data IDs, which affects regular-output + # counts, but not logs or metadata. self.check_pipetasks(["forcedPhotCcd"], len(self._raws), len(self._raws)) - # Despite the two detectors with SFM astrometric failures, the external - # calibration files still exist for them, so the forced_src catalogs - # should indeed exist for all detectors. This is not true for the - # final PSF modeling failures, which cause the PVI not to be created - # and forced photometry to skip with NoWorkFound. self.check_sources( ["forced_src"], - len(self._raws), + len(self._raws - self._forced_astrom_failures), self._min_sources, additional_checks=[self.check_aperture_corrections], # We only measure psfFlux in single-detector forced photometry. aperture_algorithms=("base_PsfFlux", ), ) self.check_datasets(["forced_src_schema"], 1) - self.check_datasets(["forced_src"], len(self._raws)) + self.check_datasets(["forced_src"], len(self._raws - self._forced_astrom_failures)) def test_forced_phot_coadd(self): """Test existence of forced photometry tables (objects).""" @@ -422,18 +439,17 @@ def test_forced_phot_coadd(self): def test_forced_phot_diffim(self): """Test existence of forced photometry tables (diffim).""" + # Lack of PVI (from reprocessVisitImage) leads to NoWorkFound for the + # forced astrometry failure data IDs, which affects regular-output + # counts, but not logs or metadata. self.check_pipetasks( ["forcedPhotDiffim", "forcedPhotCcdOnDiaObjects", "forcedPhotDiffOnDiaObjects"], len(self._raws), len(self._raws), ) - # External calibrations are applied to the diffim forced measurements, - # so the astrometry failures don't reduce the counts, but the PSF model - # failures do. - # forced source counts depend on detector/tract overlap. self.check_sources( ["forced_diff", "forced_diff_diaObject"], - len(self._raws - self._insufficient_template_coverage_failures), + len(self._raws - self._insufficient_template_coverage_failures - self._forced_astrom_failures), self._min_diasources ) self.check_datasets(["forced_diff_schema", "forced_diff_diaObject_schema"], 1) @@ -460,12 +476,12 @@ def test_image_difference(self): len(self._raws), len(self._raws) ) - # External calibrations are applied to the diffim forced measurements, - # so the astrometry failures don't reduce the counts, but the PSF model - # failures do. + # Lack of PVI (from reprocessVisitImage) leads to NoWorkFound for the + # forced astrometry failure data IDs, which affects regular-output + # counts, but not logs or metadata. self.check_datasets( ["goodSeeingDiff_differenceExp"], - len(self._raws - self._insufficient_template_coverage_failures) + len(self._raws - self._insufficient_template_coverage_failures - self._forced_astrom_failures) ) self.check_datasets(["goodSeeingDiff_diaSrc_schema"], 1) @@ -492,19 +508,19 @@ def test_forced_source_tables(self): 1, 1 ) - # External calibrations are applied to the diffim forced measurements, - # so the astrometry failures don't reduce the counts, but the PSF model - # failures do. + # Lack of PVI (from reprocessVisitImage) leads to NoWorkFound for the + # forced astrometry failure data IDs, which affects regular-output + # counts, but not logs or metadata. self.check_sources( ["forced_diff_diaObject"], - len(self._raws - self._insufficient_template_coverage_failures), + len(self._raws - self._insufficient_template_coverage_failures - self._forced_astrom_failures), self._min_diasources ) # There are fewer forced sources - self.check_sources(["forced_src_diaObject"], len(self._raws), + self.check_sources(["forced_src_diaObject"], len(self._raws - self._forced_astrom_failures), self._min_diasources) self.check_datasets(["forced_diff_diaObject_schema", "forced_src_diaObject_schema"], 1) - self.check_datasets(["forced_src_diaObject"], len(self._raws)) + self.check_datasets(["forced_src_diaObject"], len(self._raws - self._forced_astrom_failures)) def test_skymap(self): """Test existence of skymap.""" From e836c3b4898bebc84ef7c39350b6bc7a6682490c Mon Sep 17 00:00:00 2001 From: Jim Bosch Date: Fri, 3 Jan 2025 14:53:38 -0500 Subject: [PATCH 2/3] Tune config and update data ID for forced astrometry failure. --- bin/pipeline.sh | 2 +- python/lsst/ci/hsc/gen3/data.py | 4 ++-- tests/test_astrometryFail.py | 5 +++-- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/bin/pipeline.sh b/bin/pipeline.sh index b5971e8..f185a2d 100755 --- a/bin/pipeline.sh +++ b/bin/pipeline.sh @@ -58,7 +58,7 @@ pipetask --long-log --log-level="$loglevel" qgraph \ -b "$repo"/butler.yaml \ --input "$INPUTCOLL" --output "$COLLECTION" \ -p "$DRP_PIPE_DIR/pipelines/HSC/DRP-ci_hsc.yaml" \ - -c calibrateImage:astrometry.maxMeanDistanceArcsec=0.020 \ + -c calibrateImage:astrometry.maxMeanDistanceArcsec=0.02398 \ -c makeDirectWarp:select.maxPsfTraceRadiusDelta=0.2 \ --save-qgraph "$QGRAPH_FILE" diff --git a/python/lsst/ci/hsc/gen3/data.py b/python/lsst/ci/hsc/gen3/data.py index f1b326c..19c2500 100644 --- a/python/lsst/ci/hsc/gen3/data.py +++ b/python/lsst/ci/hsc/gen3/data.py @@ -54,12 +54,12 @@ {'visit': 903988, 'detector': 24, 'physical_filter': 'HSC-I'}, ] # The following lists the dataIds that fail the astrometry check with -# the config override calibrate.astrometry.maxMeanDistanceArcsec=0.020 +# the config override calibrateImage.astrometry.maxMeanDistanceArcsec=0.02398 # set. This list is sensitive to the astrometry algorithms and dataset # under consideration, so may require updating if either of those change # in the context of this repository. ASTROMETRY_FAILURE_DATA_IDS = [ - {'visit': 903344, 'detector': 0, 'physical_filter': 'HSC-R'}, + {'visit': 903988, 'detector': 23, 'physical_filter': 'HSC-I'}, ] # The following lists the dataIds that fail the PSF Model robustness check # with the config override makeWarp.select.maxPsfTraceRadiusDelta=0.2 set. diff --git a/tests/test_astrometryFail.py b/tests/test_astrometryFail.py index e8f6204..895d8b7 100644 --- a/tests/test_astrometryFail.py +++ b/tests/test_astrometryFail.py @@ -27,6 +27,7 @@ from lsst.daf.butler import Butler, DataCoordinate from lsst.utils import getPackageDir +from lsst.ci.hsc.gen3 import ASTROMETRY_FAILURE_DATA_IDS class TestAstrometryFails(lsst.utils.tests.TestCase): @@ -38,8 +39,8 @@ def setUp(self): # The dataId here represents one of the astrometry fit failures # imposed by setting astrometry.maxMeanDistanceArcsec: 0.020 in # the pipeline. - self.detector = 0 - self.visit = 903344 + self.detector = ASTROMETRY_FAILURE_DATA_IDS[0]["detector"] + self.visit = ASTROMETRY_FAILURE_DATA_IDS[0]["visit"] self.calexpMinimalDataId = DataCoordinate.standardize( instrument="HSC", detector=self.detector, visit=self.visit, universe=self.butler.dimensions, From fde205d4a4d49821090ce8a38b7f4fcfad52252b Mon Sep 17 00:00:00 2001 From: Jim Bosch Date: Mon, 6 Jan 2025 15:45:02 -0500 Subject: [PATCH 3/3] Fix bug in test code. Since the string literal "id" is never equal to an integer, these tests were definitely not working as intended before. --- tests/test_astrometryFail.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/test_astrometryFail.py b/tests/test_astrometryFail.py index 895d8b7..7052c0d 100644 --- a/tests/test_astrometryFail.py +++ b/tests/test_astrometryFail.py @@ -113,10 +113,11 @@ def testVisitCoordsAreNanForFailedAstrom(self): self.assertTrue(np.all(np.isfinite(visitTable["ra"]))) visitSummary = self.butler.get("visitSummary", self.calexpMinimalDataId) - self.assertTrue(np.isfinite(visitSummary["id" == self.detector]["astromOffsetMean"])) - self.assertTrue(np.isfinite(visitSummary["id" == self.detector]["astromOffsetStd"])) - self.assertTrue(np.all(np.isnan(visitSummary["id" == 1]["raCorners"]))) - self.assertTrue(np.all(np.isnan(visitSummary["id" == 1]["decCorners"]))) + row = visitSummary.find(self.detector) + self.assertTrue(np.isfinite(row["astromOffsetMean"])) + self.assertTrue(np.isfinite(row["astromOffsetStd"])) + self.assertTrue(np.all(np.isnan(row["raCorners"]))) + self.assertTrue(np.all(np.isnan(row["decCorners"]))) def testMetadataForFailedAstrom(self): """Test that the metadata for a failed astrometic fit is set properly.