From a686ea426ce8e5f0601038e54a5c02ad1d648c12 Mon Sep 17 00:00:00 2001
From: Yusra AlSayyad <yusra@astro.princeton.edu>
Date: Wed, 26 Jun 2024 15:07:41 -0700
Subject: [PATCH] Raise PhotoCalInputFluxError if no calib fluxes for photocal

---
 python/lsst/pipe/tasks/photoCal.py | 42 ++++++++++++++++++++++++++----
 tests/test_photoCal.py             | 20 +++++++++++++-
 2 files changed, 56 insertions(+), 6 deletions(-)

diff --git a/python/lsst/pipe/tasks/photoCal.py b/python/lsst/pipe/tasks/photoCal.py
index 57118e426..ebc291c80 100644
--- a/python/lsst/pipe/tasks/photoCal.py
+++ b/python/lsst/pipe/tasks/photoCal.py
@@ -19,7 +19,7 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <https://www.gnu.org/licenses/>.
 
-__all__ = ["PhotoCalTask", "PhotoCalConfig"]
+__all__ = ["PhotoCalTask", "PhotoCalConfig", "PhotoCalInputFluxError"]
 
 import math
 import sys
@@ -38,6 +38,35 @@
 from .colorterms import ColortermLibrary
 
 
+class PhotoCalInputFluxError(pipeBase.AlgorithmError):
+    """Raised if photoCal fails in a non-recoverable way.
+
+    Parameters
+    ----------
+    nMatches : `int`
+        Number of nMatches available to the fitter at the point of failure.
+    nFiniteInstFluxes : `int`
+        Number of calibration instFluxes that are are finite (non-NaN).
+    nFiniteInstFluxes : `int`
+        Number of calibration instFluxErrs that are are finite (non-NaN).
+    """
+    def __init__(self, *, nMatches, nFiniteInstFluxes, nFiniteInstFluxErrs):
+        msg = (f"No finite calibration instFluxes ({nFiniteInstFluxes}) or "
+               f"instFluxErrs ({nFiniteInstFluxErrs}) for {nMatches} matches.")
+        super().__init__(msg)
+        self.nMatches = nMatches
+        self.nFiniteInstFluxes = nFiniteInstFluxes
+        self.nFiniteInstFluxErrs = nFiniteInstFluxErrs
+
+    @property
+    def metadata(self):
+        metadata = {"nMatches": self.nMatches,
+                    "nFiniteInstFluxes": self.nFiniteInstFluxes,
+                    "nFiniteInstFluxErrs": self.nFiniteInstFluxErrs,
+                    }
+        return metadata
+
+
 class PhotoCalConfig(pexConf.Config):
     """Config for PhotoCal."""
 
@@ -238,10 +267,13 @@ def extractMagArrays(self, matches, filterLabel, sourceKeys):
         """
         srcInstFluxArr = np.array([m.second.get(sourceKeys.instFlux) for m in matches])
         srcInstFluxErrArr = np.array([m.second.get(sourceKeys.instFluxErr) for m in matches])
-        if not np.all(np.isfinite(srcInstFluxErrArr)):
-            # this is an unpleasant hack; see DM-2308 requesting a better solution
-            self.log.warning("Source catalog does not have flux uncertainties; using sqrt(flux).")
-            srcInstFluxErrArr = np.sqrt(srcInstFluxArr)
+
+        nFiniteInstFluxes = np.isfinite(srcInstFluxArr).sum()
+        nFiniteInstFluxErrs = np.isfinite(srcInstFluxErrArr).sum()
+
+        if not nFiniteInstFluxes or not nFiniteInstFluxErrs:
+            raise PhotoCalInputFluxError(nMatches=len(matches), nFiniteInstFluxes=nFiniteInstFluxes,
+                                         nFiniteInstFluxErrs=nFiniteInstFluxErrs)
 
         # convert source instFlux from DN to an estimate of nJy
         referenceFlux = (0*u.ABmag).to_value(u.nJy)
diff --git a/tests/test_photoCal.py b/tests/test_photoCal.py
index 2fd93ee78..2d2fa7ebb 100755
--- a/tests/test_photoCal.py
+++ b/tests/test_photoCal.py
@@ -32,7 +32,7 @@
 import lsst.afw.image as afwImage
 import lsst.utils.tests
 from lsst.utils import getPackageDir
-from lsst.pipe.tasks.photoCal import PhotoCalTask, PhotoCalConfig
+from lsst.pipe.tasks.photoCal import PhotoCalTask, PhotoCalConfig, PhotoCalInputFluxError
 from lsst.pipe.tasks.colorterms import Colorterm, ColortermDict, ColortermLibrary
 from lsst.utils.logging import TRACE
 from lsst.meas.algorithms.testUtils import MockReferenceObjectLoaderFromFiles
@@ -199,6 +199,24 @@ def testColorTerms(self):
         # zeropoint: 32.3145
         self.assertLess(abs(self.zp - (31.3145 + zeroPointOffset)), 0.05)
 
+    def testNoFiniteFluxes(self):
+        """Test case where matches exist but calib fluxes are NaN"""
+        catalog = self.srcCat.copy(deep=True)
+        catalog['slot_ApFlux_instFlux'] = np.nan
+        task = PhotoCalTask(self.refObjLoader, config=self.config, schema=self.srcCat.schema)
+        with self.assertRaisesRegex(PhotoCalInputFluxError,
+                                    r"No finite calibration instFluxes \(0\) or instFluxErrs \(\d+\)"):
+            task.run(exposure=self.exposure, sourceCat=catalog)
+
+    def testNoFiniteFluxErrs(self):
+        """Test case where matches exist but calib fluxErrs are NaN"""
+        catalog = self.srcCat.copy(deep=True)
+        catalog['slot_ApFlux_instFluxErr'] = np.nan
+        task = PhotoCalTask(self.refObjLoader, config=self.config, schema=self.srcCat.schema)
+        with self.assertRaisesRegex(PhotoCalInputFluxError,
+                                    r"No finite calibration instFluxes \(\d+\) or instFluxErrs \(0\)"):
+            task.run(exposure=self.exposure, sourceCat=catalog)
+
 
 class MemoryTester(lsst.utils.tests.MemoryTestCase):
     pass