Merge pull request #103 from lsst/tickets/DM-15241

DM-15241 log error on large chi2

python/lsst/jointcal/jointcal.py

@@ -648,7 +648,6 @@ def _fit_photometry(self, associations, dataName=None):
 
         chi2 = self._iterate_fit(associations,
                                  fit,
-                                 model,
                                  self.config.maxPhotometrySteps,
                                  "photometry",
                                  "Model Fluxes",
@@ -737,7 +736,6 @@ def _fit_astrometry(self, associations, dataName=None):
 
         chi2 = self._iterate_fit(associations,
                                  fit,
-                                 model,
                                  self.config.maxAstrometrySteps,
                                  "astrometry",
                                  "Distortions Positions",
@@ -761,36 +759,41 @@ def _check_stars(self, associations):
                 self.log.warn("ccdImage %s has only %s RefStars (desired %s)",
                               ccdImage.getName(), nRefStars, self.config.minRefStarsPerCcd)
 
-    def _iterate_fit(self, associations, fit, model, max_steps, name, whatToFit, doRankUpdate=True,
+    def _iterate_fit(self, associations, fit, max_steps, name, whatToFit, doRankUpdate=True,
                      doLineSearch=False):
         """Run fit.minimize up to max_steps times, returning the final chi2."""
 
         dumpMatrixFile = "%s_postinit" % name if self.config.writeInitMatrix else ""
         for i in range(max_steps):
             # outlier removal at 5 sigma.
-            r = fit.minimize(whatToFit,
-                             self.config.outlierRejectSigma,
-                             doRankUpdate=doRankUpdate,
-                             doLineSearch=doLineSearch,
-                             dumpMatrixFile=dumpMatrixFile)
+            result = fit.minimize(whatToFit,
+                                  self.config.outlierRejectSigma,
+                                  doRankUpdate=doRankUpdate,
+                                  doLineSearch=doLineSearch,
+                                  dumpMatrixFile=dumpMatrixFile)
             dumpMatrixFile = ""  # clear it so we don't write the matrix again.
             chi2 = fit.computeChi2()
             self._check_stars(associations)
             if not np.isfinite(chi2.chi2):
                 raise FloatingPointError('Fit iteration chi2 is invalid: %s'%chi2)
             self.log.info(str(chi2))
-            if r == MinimizeResult.Converged:
+            if result == MinimizeResult.Converged:
                 if doRankUpdate:
                     self.log.debug("fit has converged - no more outliers - redo minimization "
                                    "one more time in case we have lost accuracy in rank update.")
                     # Redo minimization one more time in case we have lost accuracy in rank update
-                    r = fit.minimize(whatToFit, 5)  # outliers removal at 5 sigma.
+                    result = fit.minimize(whatToFit, 5)  # outliers removal at 5 sigma.
                 chi2 = fit.computeChi2()
                 self.log.info("Fit completed with: %s", str(chi2))
+
+                # log a message for a large final chi2, TODO: DM-15247 for something better
+                if chi2.chi2/chi2.ndof >= 4.0:
+                    self.log.error("Potentially bad fit: High chi-squared/ndof.")
+
                 break
-            elif r == MinimizeResult.Chi2Increased:
+            elif result == MinimizeResult.Chi2Increased:
                 self.log.warn("still some ouliers but chi2 increases - retry")
-            elif r == MinimizeResult.Failed:
+            elif result == MinimizeResult.Failed:
                 raise RuntimeError("Chi2 minimization failure, cannot complete fit.")
             else:
                 raise RuntimeError("Unxepected return code from minimize().")

tests/test_jointcal.py

@@ -0,0 +1,99 @@
+# See COPYRIGHT file at the top of the source tree.
+import unittest
+import unittest.mock
+
+import lsst.log
+import lsst.utils
+
+import lsst.jointcal
+from lsst.jointcal import MinimizeResult
+import lsst.jointcal.chi2
+import lsst.jointcal.testUtils
+
+
+# for MemoryTestCase
+def setup_module(module):
+    lsst.utils.tests.init()
+
+
+class TestJointcalIterateFit(lsst.utils.tests.TestCase):
+    def setUp(self):
+        struct = lsst.jointcal.testUtils.createTwoFakeCcdImages(100, 100)
+        self.ccdImageList = struct.ccdImageList
+        # so that countStars() returns nonzero results
+        for ccdImage in self.ccdImageList:
+            ccdImage.resetCatalogForFit()
+
+        self.config = lsst.jointcal.jointcal.JointcalConfig()
+        # disable both, so it doesn't configure any refObjLoaders
+        self.config.doAstrometry = False
+        self.config.doPhotometry = False
+        self.jointcal = lsst.jointcal.JointcalTask(config=self.config)
+
+        self.goodChi2 = lsst.jointcal.chi2.Chi2Statistic()
+        # chi2/ndof == 2.0 should be non-bad
+        self.goodChi2.chi2 = 200.0
+        self.goodChi2.ndof = 100
+
+        self.badChi2 = lsst.jointcal.chi2.Chi2Statistic()
+        self.badChi2.chi2 = 600.0
+        self.badChi2.ndof = 100
+
+        self.maxSteps = 20
+        self.name = "testing"
+        self.whatToFit = ""  # unneeded, since we're mocking the fitter
+
+        # Mock the fitter and association manager, so we can force particular
+        # return values/exceptions. Default to "good" return values.
+        self.fitter = unittest.mock.Mock(spec=lsst.jointcal.PhotometryFit)
+        self.fitter.computeChi2.return_value = self.goodChi2
+        self.fitter.minimize.return_value = MinimizeResult.Converged
+        self.associations = unittest.mock.Mock(spec=lsst.jointcal.Associations)
+        self.associations.getCcdImageList.return_value = self.ccdImageList
+
+    def test_iterateFit_success(self):
+        chi2 = self.jointcal._iterate_fit(self.associations, self.fitter,
+                                          self.maxSteps, self.name, self.whatToFit)
+        self.assertEqual(chi2, self.goodChi2)
+        # Once for the for loop, the second time for the rank update.
+        self.assertEqual(self.fitter.minimize.call_count, 2)
+
+    def test_iterateFit_failed(self):
+        self.fitter.minimize.return_value = MinimizeResult.Failed
+
+        with self.assertRaises(RuntimeError):
+            self.jointcal._iterate_fit(self.associations, self.fitter,
+                                       self.maxSteps, self.name, self.whatToFit)
+        self.assertEqual(self.fitter.minimize.call_count, 1)
+
+    def test_iterateFit_badFinalChi2(self):
+        log = unittest.mock.Mock(spec=lsst.log.Log)
+        self.jointcal.log = log
+        self.fitter.computeChi2.return_value = self.badChi2
+
+        chi2 = self.jointcal._iterate_fit(self.associations, self.fitter,
+                                          self.maxSteps, self.name, self.whatToFit)
+        self.assertEqual(chi2, self.badChi2)
+        log.info.assert_called_with("Fit completed with: %s", str(self.badChi2))
+        log.error.assert_called_with("Potentially bad fit: High chi-squared/ndof.")
+
+    def test_iterateFit_exceedMaxSteps(self):
+        log = unittest.mock.Mock(spec=lsst.log.Log)
+        self.jointcal.log = log
+        self.fitter.minimize.return_value = MinimizeResult.Chi2Increased
+        maxSteps = 3
+
+        chi2 = self.jointcal._iterate_fit(self.associations, self.fitter,
+                                          maxSteps, self.name, self.whatToFit)
+        self.assertEqual(chi2, self.goodChi2)
+        self.assertEqual(self.fitter.minimize.call_count, maxSteps)
+        log.error.assert_called_with("testing failed to converge after %s steps" % maxSteps)
+
+
+class MemoryTester(lsst.utils.tests.MemoryTestCase):
+    pass
+
+
+if __name__ == "__main__":
+    lsst.utils.tests.init()
+    unittest.main()