Merge pull request #19 from lsst/tickets/DM-31345

DM-31345: Fix the underlying bug resulting in PSF-matching failures in ci_hsc and ci_imsim

python/lsst/meas/extensions/gaap/_gaap.py

@@ -28,6 +28,7 @@
 from typing import Generator, Optional, Union
 from functools import partial
 import itertools
+import logging
 import lsst.afw.detection as afwDetection
 import lsst.afw.image as afwImage
 import lsst.afw.geom as afwGeom
@@ -267,6 +268,9 @@ class BaseGaapFluxMixin:
     schema : `lsst.afw.table.Schema`
         The schema for the measurement output catalog. New fields will be added
         to hold measurements produced by this plugin.
+    logName : `str`, optional
+        Name to use when logging errors. This is typically provided by the
+        measurement framework.
 
     Raises
     ------
@@ -277,6 +281,7 @@ class BaseGaapFluxMixin:
     """
 
     ConfigClass = BaseGaapFluxConfig
+    hasLogName = True
 
     def __init__(self, config: BaseGaapFluxConfig, name, schema, logName=None) -> None:
         # Flag definitions for each variant of GAaP measurement
@@ -331,6 +336,7 @@ def __init__(self, config: BaseGaapFluxConfig, name, schema, logName=None) -> No
             flagName = self.ConfigClass._getGaapResultName(scalingFactor, "flag_gaussianization")
             flagDefs.add(flagName, "PSF Gaussianization failed when trying to scale by this factor.")
 
+        self.log = logging.getLogger(logName)
         self.flagHandler = measBase.FlagHandler.addFields(schema, name, flagDefs)
         self.EdgeFlagKey = schema.addField(schema.join(name, "flag_edge"), type="Flag",
                                            doc="Source is too close to the edge")
@@ -549,10 +555,10 @@ def _gaussianizeAndMeasure(self, measRecord: lsst.afw.table.SourceRecord,
             raise measBase.FatalAlgorithmError("No PSF in exposure")
         wcs = exposure.getWcs()
 
-        seeing = psf.computeShape(center).getTraceRadius()
+        psfSigma = psf.computeShape(center).getDeterminantRadius()
         errorCollection = dict()
         for scalingFactor in self.config.scalingFactors:
-            targetSigma = scalingFactor*seeing
+            targetSigma = scalingFactor*psfSigma
             # If this target PSF is bound to fail for all apertures,
             # set the flags and move on without PSF Gaussianization.
             if self._isAllFailure(measRecord, scalingFactor, targetSigma):
@@ -695,6 +701,9 @@ def fail(self, measRecord, error=None):
             Error causing failure, or `None`.
         """
         if error is not None:
+            center = measRecord.getCentroid()
+            self.log.error("Failed to solve for PSF matching kernel in GAaP for (%f, %f): %s",
+                           center.getX(), center.getY(), error)
             for scalingFactor in error.errorDict:
                 flagName = self.ConfigClass._getGaapResultName(scalingFactor, "flag_gaussianization",
                                                                self.name)
@@ -727,7 +736,8 @@ class SingleFrameGaapFluxPlugin(BaseGaapFluxMixin, measBase.SingleFramePlugin):
     metadata : `lsst.daf.base.PropertySet`
         Plugin metadata that will be attached to the output catalog.
     logName : `str`, optional
-        Name to use when logging errors.
+        Name to use when logging errors. This will be provided by the
+        measurement framework.
 
     Notes
     -----
@@ -784,7 +794,8 @@ class ForcedGaapFluxPlugin(BaseGaapFluxMixin, measBase.ForcedPlugin):
     metadata : `lsst.daf.base.PropertySet`
         Plugin metadata that will be attached to the output catalog.
     logName : `str`, optional
-        Name to use when logging errors.
+        Name to use when logging errors. This will be provided by the
+        measurement framework.
     """
     ConfigClass = ForcedGaapFluxConfig
 

python/lsst/meas/extensions/gaap/_gaussianizePsf.py

@@ -118,8 +118,9 @@ def run(self, exposure: lsst.afw.image.Exposure, center: lsst.geom.Point2D,  # n
         result = self._buildCellSet(exposure, center, targetPsfModel)
         kernelCellSet = result.kernelCellSet
         targetPsfModel = result.targetPsfModel
-        fwhmScience = exposure.getPsf().computeShape().getDeterminantRadius()*sigma2fwhm
+        fwhmScience = exposure.getPsf().computeShape(center).getDeterminantRadius()*sigma2fwhm
         fwhmModel = targetPsfModel.computeShape().getDeterminantRadius()*sigma2fwhm
+        self.log.debug("Ratio of GAaP model to science PSF = %f", fwhmModel/fwhmScience)
 
         basisList = makeKernelBasisList(self.kConfig, fwhmScience, fwhmModel,
                                         basisSigmaGauss=basisSigmaGauss,

tests/test_gaap.py

@@ -122,7 +122,7 @@ def makeAlgorithm(self, gaapConfig=None):
         if gaapConfig is None:
             gaapConfig = lsst.meas.extensions.gaap.SingleFrameGaapFluxConfig()
         gaapPlugin = lsst.meas.extensions.gaap.SingleFrameGaapFluxPlugin(gaapConfig,
-                                                                         'ext_gaap_GaapFlux',
+                                                                         "ext_gaap_GaapFlux",
                                                                          schema, None)
         if gaapConfig.doOptimalPhotometry:
             afwTable.QuadrupoleKey.addFields(schema, "psfShape", "PSF shape")
@@ -236,7 +236,8 @@ def testFail(self, scalingFactors=[100.], sigmas=[500.]):
         """Test that the fail method sets the flags correctly.
 
         Set config parameters that are guaranteed to raise exceptions,
-        and check that they are handled properly by the `fail` method.
+        and check that they are handled properly by the `fail` method and that
+        expected log messages are generated.
         For failure modes not handled by the `fail` method, we test them
         in the ``testFlags`` method.
         """
@@ -257,7 +258,24 @@ def testFail(self, scalingFactors=[100.], sigmas=[500.]):
                                                       algMetadata=algMetadata)
         exposure, catalog = self.dataset.realize(0.0, sfmTask.schema)
         self.recordPsfShape(catalog)
-        sfmTask.run(catalog, exposure)
+
+        # Expected error messages in the logs when running `sfmTask`.
+        errorMessage = [("ERROR:measurement.ext_gaap_GaapFlux:"
+                         "Failed to solve for PSF matching kernel in GAaP for (100.000000, 670.000000): "
+                         "Problematic scaling factors = 100.0 "
+                         "Errors: Exception('Unable to determine kernel sum; 0 candidates')"),
+                        ("ERROR:measurement.ext_gaap_GaapFlux:"
+                         "Failed to solve for PSF matching kernel in GAaP for (100.000000, 870.000000): "
+                         "Problematic scaling factors = 100.0 "
+                         "Errors: Exception('Unable to determine kernel sum; 0 candidates')"),
+                        ("ERROR:measurement.ext_gaap_GaapFlux:"
+                         "Failed to solve for PSF matching kernel in GAaP for (-10.000000, -20.000000): "
+                         "Problematic scaling factors = 100.0 "
+                         "Errors: Exception('Unable to determine kernel sum; 0 candidates')")]
+
+        with self.assertLogs(sfmTask.log.name, "ERROR") as cm:
+            sfmTask.run(catalog, exposure)
+        self.assertEqual(cm.output, errorMessage)
 
         for record in catalog:
             self.assertFalse(record[algName + "_flag"])