Merge pull request #1946 from abhisrkckl/downhill

Change `StepProblem` and `MaxIterReached` into warnings

Author: dlakaplan

CHANGELOG-unreleased.md

@@ -9,6 +9,7 @@ the released changes.
 
 ## Unreleased
 ### Changed
+- Change `StepProblem` and `MaxIterReached` into warnings
 ### Added
 - Anderson-Darling test for normal data with fixed mean/variance
 - KS test to check if the whitened residuals are unit-normal distributed

src/pint/exceptions.py

@@ -27,7 +27,7 @@ class DegeneracyWarning(UserWarning):
     pass
 
 
-class ConvergenceFailure(ValueError):
+class ConvergenceFailure(UserWarning):
     pass
 
 

src/pint/fitter.py

@@ -60,9 +60,9 @@
 
 import contextlib
 import copy
+from functools import cached_property
 from typing import Dict, List, Literal, Optional, Tuple, Union
 from warnings import warn
-from functools import cached_property
 
 import astropy.units as u
 import numpy as np
@@ -72,21 +72,16 @@
 from numdifftools import Hessian
 
 import pint
-from pint.models.timing_model import TimingModel
 from pint.exceptions import (
     ConvergenceFailure,
     CorrelatedErrors,
     DegeneracyWarning,
+    InvalidModelParameters,
     MaxiterReached,
     StepProblem,
 )
-from pint.models.parameter import (
-    AngleParameter,
-    InvalidModelParameters,
-    Parameter,
-    boolParameter,
-    strParameter,
-)
+from pint.models.parameter import AngleParameter, Parameter, boolParameter, strParameter
+from pint.models.timing_model import TimingModel
 from pint.pint_matrix import (
     CorrelationMatrix,
     CovarianceMatrix,
@@ -944,7 +939,7 @@ def _fit_toas(
         maxiter=20,
         required_chi2_decrease=1e-2,
         max_chi2_increase=1e-2,
-        min_lambda=1e-3,
+        min_lambda=1e-4,
         debug=False,
     ) -> bool:
         """Downhill fit implementation for fitting the timing model parameters.
@@ -955,12 +950,11 @@ def _fit_toas(
         # setup
         self.model.validate()
         self.model.validate_toas(self.toas)
+
         current_state = self.create_state()
         best_state = current_state
         self.converged = False
-        # algorithm
         exception = None
-
         for i in range(maxiter):
             step = current_state.step
             lambda_ = 1
@@ -973,14 +967,10 @@ def _fit_toas(
                         best_state = new_state
                     if chi2_decrease < -max_chi2_increase:
                         raise InvalidModelParameters(
-                            f"chi2 increased from {current_state.chi2} to {new_state.chi2} "
-                            f"when trying to take a step with lambda {lambda_}"
+                            f"chi2 increased from {current_state.chi2} to {new_state.chi2} when trying to take a step with lambda {lambda_}"
                         )
                     log.trace(
-                        f"Iteration {i}: "
-                        f"Updating state, chi2 goes down by {chi2_decrease} "
-                        f"from {current_state.chi2} "
-                        f"to {new_state.chi2}"
+                        f"Iteration {i}: Updating state, chi2 goes down by {chi2_decrease} from {current_state.chi2} to {new_state.chi2}"
                     )
                     exception = None
                     current_state = new_state
@@ -989,13 +979,9 @@ def _fit_toas(
                     # This could be an exception evaluating new_state.chi2 or an increase in value
                     # If bad parameter values escape, look in ModelState.resids for the except
                     # that should catch them
-                    lambda_ /= 2
+                    lambda_ /= 1.5
                     log.trace(f"Iteration {i}: Shortening step to {lambda_}: {e}")
                     if lambda_ < min_lambda:
-                        log.warning(
-                            f"Unable to improve chi2 even with very small steps, stopping "
-                            f"but keeping best state, message was: {e}"
-                        )
                         exception = e
                         break
             if (
@@ -1042,11 +1028,11 @@ def _fit_toas(
         self.update_model(self.current_state.chi2)
 
         if exception is not None:
-            raise StepProblem(
-                "Unable to improve chi2 even with very small steps"
-            ) from exception
+            warn("Unable to improve chi2 even with very small steps", StepProblem)
+            return False
+
         if not self.converged:
-            raise MaxiterReached(f"Convergence not detected after {maxiter} steps.")
+            warn(f"Convergence not detected after {maxiter} steps.", MaxiterReached)
 
         return self.converged
 

src/pint/models/parameter.py

@@ -33,7 +33,6 @@
 from uncertainties import ufloat
 
 from pint import pint_units
-from pint.exceptions import InvalidModelParameters
 from pint.models import priors
 from pint.observatory import get_observatory
 from pint.pulsar_mjd import (

src/pint/models/stand_alone_psr_binaries/DD_model.py

@@ -5,7 +5,7 @@
 import numpy as np
 
 from pint import Tsun
-from pint.models.parameter import InvalidModelParameters
+from pint.exceptions import InvalidModelParameters
 
 from .binary_generic import PSR_BINARY
 

src/pint/models/stand_alone_psr_binaries/ELL1_model.py

@@ -4,7 +4,7 @@
 import astropy.units as u
 import numpy as np
 
-from pint.models.parameter import InvalidModelParameters
+from pint.exceptions import InvalidModelParameters
 
 from .binary_generic import PSR_BINARY
 

tests/test_downhill_fitter.py

@@ -177,15 +177,15 @@ def test_wls_two_step(model_eccentric_toas):
 
     f = pint.fitter.DownhillWLSFitter(toas, model_wrong)
     f.model.free_params = ["ECC"]
-    with pytest.raises(pint.fitter.MaxiterReached):
+    with pytest.warns(pint.fitter.MaxiterReached):
         f.fit_toas(maxiter=2)
     assert not f.converged
 
     f2 = pint.fitter.DownhillWLSFitter(toas, model_wrong)
     f2.model.free_params = ["ECC"]
-    with pytest.raises(pint.fitter.MaxiterReached):
+    with pytest.warns(pint.fitter.MaxiterReached):
         f2.fit_toas(maxiter=1)
-    with pytest.raises(pint.fitter.MaxiterReached):
+    with pytest.warns(pint.fitter.MaxiterReached):
         f2.fit_toas(maxiter=1)
     assert np.abs(f.model.ECC.value - f2.model.ECC.value) < 1e-12
 
@@ -198,14 +198,14 @@ def test_gls_two_step(model_eccentric_toas_ecorr, full_cov):
 
     f = pint.fitter.DownhillGLSFitter(toas, model_wrong)
     f.model.free_params = ["ECC"]
-    with pytest.raises(pint.fitter.MaxiterReached):
+    with pytest.warns(pint.fitter.MaxiterReached):
         f.fit_toas(maxiter=2, full_cov=full_cov)
     assert not f.converged
     f2 = pint.fitter.DownhillGLSFitter(toas, model_wrong)
     f2.model.free_params = ["ECC"]
-    with pytest.raises(pint.fitter.MaxiterReached):
+    with pytest.warns(pint.fitter.MaxiterReached):
         f2.fit_toas(maxiter=1, full_cov=full_cov)
-    with pytest.raises(pint.fitter.MaxiterReached):
+    with pytest.warns(pint.fitter.MaxiterReached):
         f2.fit_toas(maxiter=1, full_cov=full_cov)
     assert np.abs(f.model.ECC.value - f2.model.ECC.value) < 1e-12
 
@@ -218,14 +218,14 @@ def test_wb_two_step(model_eccentric_toas_wb, full_cov):
 
     f = pint.fitter.WidebandDownhillFitter(toas, model_wrong)
     f.model.free_params = ["ECC"]
-    with pytest.raises(pint.fitter.MaxiterReached):
+    with pytest.warns(pint.fitter.MaxiterReached):
         f.fit_toas(maxiter=2, full_cov=full_cov)
     assert not f.converged
     f2 = pint.fitter.WidebandDownhillFitter(toas, model_wrong)
     f2.model.free_params = ["ECC"]
-    with pytest.raises(pint.fitter.MaxiterReached):
+    with pytest.warns(pint.fitter.MaxiterReached):
         f2.fit_toas(maxiter=1, full_cov=full_cov)
-    with pytest.raises(pint.fitter.MaxiterReached):
+    with pytest.warns(pint.fitter.MaxiterReached):
         f2.fit_toas(maxiter=1, full_cov=full_cov)
     # FIXME: The full_cov version differs at the 1e-10 level for some reason, is it a failure really?
     assert np.abs(f.model.ECC.value - f2.model.ECC.value) < 1e-9