diff --git a/autofit/non_linear/samples/interface.py b/autofit/non_linear/samples/interface.py index d2aad2abd..70e674a92 100644 --- a/autofit/non_linear/samples/interface.py +++ b/autofit/non_linear/samples/interface.py @@ -2,44 +2,73 @@ from functools import wraps from typing import Union, List, Tuple, Dict +from autofit import exc from autofit.mapper.model import ModelInstance from autofit.mapper.prior_model.abstract import AbstractPriorModel from autofit.mapper.prior_model.abstract import Path -def to_instance(func): +def to_instance(recover: str = "raise"): """ Decorator for methods that return a vector of parameters, which can be converted to a model instance. + Constructor validation can become stricter after a result was written, so a stored + vector may be rejected via the narrow `FitException` contract when materialized. + The `recover` policy decides what happens then: + + - "raise": raise a typed `SamplesException` (chained to the rejection). This is the + only honest option for vectors with no stored sample to substitute — e.g. the + marginalized methods, which synthesize each parameter independently — and for + `from_sample_index`, where the caller asked for one specific sample. + - "next_valid": call the object's `_next_valid_instance`, which substitutes the + best valid stored sample (see `Samples._next_valid_instance`). + Parameters ---------- - func - A method that returns a vector of parameters + recover + The recovery policy applied when the model rejects the vector with + `FitException` while building an instance. Returns ------- - A wrapper that converts the vector to a model instance + A decorator whose wrapper converts the vector to a model instance """ - @wraps(func) - def wrapper( - self, - *args, - as_instance: bool = True, - as_dict: bool = False, - **kwargs, - ) -> Union[List, Dict, ModelInstance]: - vector = func(self, *args, **kwargs) - - if as_dict: - return {".".join(path[0]): value for path, value in zip(self.paths, vector)} - - if as_instance: - return self._instance_from_vector(vector) - - return vector - - return wrapper + def decorator(func): + @wraps(func) + def wrapper( + self, + *args, + as_instance: bool = True, + as_dict: bool = False, + **kwargs, + ) -> Union[List, Dict, ModelInstance]: + vector = func(self, *args, **kwargs) + + if as_dict: + return { + ".".join(path[0]): value + for path, value in zip(self.paths, vector) + } + + if as_instance: + try: + return self._instance_from_vector(vector) + except exc.FitException as error: + if recover == "next_valid": + return self._next_valid_instance(error) + raise exc.SamplesException( + f"The stored parameters returned by {func.__name__} cannot " + f"be reconstructed as a model instance because the current " + f"model rejected them (see the chained exception). Pass " + f"as_instance=False to retrieve the raw values instead." + ) from error + + return vector + + return wrapper + + return decorator class SamplesInterface(ABC): @@ -90,7 +119,7 @@ def names(self) -> List[Tuple[str]]: self._names = self.model.all_names return self._names - @to_instance + @to_instance() def max_log_likelihood(self, as_instance: bool = True) -> List[float]: """ The parameters of the maximum log likelihood sample of the `NonLinearSearch` returned as a model instance or diff --git a/autofit/non_linear/samples/mcmc.py b/autofit/non_linear/samples/mcmc.py index 7d366a6e3..c8e32e188 100644 --- a/autofit/non_linear/samples/mcmc.py +++ b/autofit/non_linear/samples/mcmc.py @@ -133,7 +133,7 @@ def converged(self) -> bool: total_samples=self.total_samples ) - @to_instance + @to_instance() def median_pdf(self, as_instance: bool = True) -> [float]: """ The median of the probability density function (PDF) of every parameter marginalized in 1D, returned @@ -150,7 +150,7 @@ def median_pdf(self, as_instance: bool = True) -> [float]: return self.max_log_likelihood(as_instance=False) - @to_instance + @to_instance() def values_at_sigma(self, sigma: float) -> [float]: """ The value of every parameter marginalized in 1D at an input sigma value of its probability density function diff --git a/autofit/non_linear/samples/pdf.py b/autofit/non_linear/samples/pdf.py index d7c99b2b5..7167170ed 100644 --- a/autofit/non_linear/samples/pdf.py +++ b/autofit/non_linear/samples/pdf.py @@ -136,7 +136,7 @@ def pdf_converged(self) -> bool: return False return True - @to_instance + @to_instance() def median_pdf(self) -> List[float]: """ The median of the probability density function (PDF) of every parameter marginalized in 1D, returned @@ -149,7 +149,7 @@ def median_pdf(self) -> List[float]: ] return self.max_log_likelihood(as_instance=False) - @to_instance + @to_instance() def values_at_sigma(self, sigma: float) -> [Tuple, ModelInstance]: """ The value of every parameter marginalized in 1D at an input sigma value of its probability density function @@ -198,7 +198,7 @@ def values_at_sigma(self, sigma: float) -> [Tuple, ModelInstance]: for index in range(len(parameters_min)) ] - @to_instance + @to_instance() def values_at_upper_sigma(self, sigma: float) -> Union[List, ModelInstance]: """ The upper value of every parameter marginalized in 1D at an input sigma value of its probability density @@ -215,7 +215,7 @@ def values_at_upper_sigma(self, sigma: float) -> Union[List, ModelInstance]: map(lambda param: param[1], self.values_at_sigma(sigma, as_instance=False)) ) - @to_instance + @to_instance() def values_at_lower_sigma(self, sigma: float) -> Union[List, ModelInstance]: """ The lower value of every parameter marginalized in 1D at an input sigma value of its probability density @@ -232,7 +232,7 @@ def values_at_lower_sigma(self, sigma: float) -> Union[List, ModelInstance]: map(lambda param: param[0], self.values_at_sigma(sigma, as_instance=False)) ) - @to_instance + @to_instance() def errors_at_sigma( self, sigma: float, as_instance: bool = True ) -> [Tuple, ModelInstance]: @@ -254,7 +254,7 @@ def errors_at_sigma( for lower, upper in zip(error_vector_lower, error_vector_upper) ] - @to_instance + @to_instance() def errors_at_upper_sigma( self, sigma: float, as_instance: bool = True ) -> Union[List, ModelInstance]: @@ -278,7 +278,7 @@ def errors_at_upper_sigma( ) ) - @to_instance + @to_instance() def errors_at_lower_sigma(self, sigma: float) -> Union[List, ModelInstance]: """ The lower error of every parameter marginalized in 1D at an input sigma value of its probability density @@ -300,7 +300,7 @@ def errors_at_lower_sigma(self, sigma: float) -> Union[List, ModelInstance]: ) ) - @to_instance + @to_instance() def error_magnitudes_at_sigma(self, sigma: float) -> Union[List, ModelInstance]: """ The magnitude of every error after marginalization in 1D at an input sigma value of the probability density @@ -397,7 +397,7 @@ def samples_drawn_randomly_via_pdf_from(self, total_draws: int = 100) -> "Sample samples_info=self.samples_info, ) - @to_instance + @to_instance() def offset_values_via_input_values( self, input_vector: List ) -> Union[List, ModelInstance]: diff --git a/autofit/non_linear/samples/samples.py b/autofit/non_linear/samples/samples.py index 26ca91ab6..59b495330 100644 --- a/autofit/non_linear/samples/samples.py +++ b/autofit/non_linear/samples/samples.py @@ -70,15 +70,15 @@ def __repr__(self): @property def instances(self): """ - One model instance for each sample + One model instance for each stored sample the current model can still build. + + Stored samples rejected via the narrow :class:`FitException` contract are + skipped (a warning is logged), so the list can be shorter than + ``sample_list``. Use ``valid_sample_instance_pairs`` when instances must + stay paired with their samples (e.g. weights). """ return [ - self.model.instance_from_vector( - sample.parameter_lists_for_paths( - self.paths if sample.is_path_kwargs else self.names - ), - ) - for sample in self.sample_list + instance for _, instance in self.valid_sample_instance_pairs() ] def valid_sample_instance_pairs( @@ -405,19 +405,38 @@ def max_log_likelihood( try: return self._instance_from_vector(vector) except exc.FitException as error: - last_error = error + return self._instance_from_next_valid_sample( + exclude_sample=sample, + sample_value=lambda candidate: candidate.log_likelihood, + quantity_name="likelihood", + last_error=error, + ) + + def _instance_from_next_valid_sample( + self, + exclude_sample: Sample, + sample_value, + quantity_name: str, + last_error: Exception, + ) -> ModelInstance: + """ + The highest-``quantity_name`` stored sample, excluding ``exclude_sample`` + (already rejected), that the current model can still reconstruct. - valid_sample_candidates = sorted( - (candidate for candidate in self.sample_list if candidate is not sample), + Raises ``SamplesException`` chained to the final rejection when no stored + sample survives. + """ + candidates = sorted( + (candidate for candidate in self.sample_list if candidate is not exclude_sample), key=lambda candidate: ( float("-inf") - if np.isnan(candidate.log_likelihood) - else candidate.log_likelihood + if np.isnan(sample_value(candidate)) + else sample_value(candidate) ), reverse=True, ) - for candidate in valid_sample_candidates: + for candidate in candidates: candidate_vector = candidate.parameter_lists_for_paths( self.paths if candidate.is_path_kwargs else self.names ) @@ -428,10 +447,11 @@ def max_log_likelihood( continue logger.warning( - "The maximum-likelihood stored sample can no longer be " - "reconstructed because the model rejected it with " - "FitException; using the highest-likelihood valid stored " - "sample instead." + "The maximum-%s stored sample can no longer be reconstructed " + "because the model rejected it with FitException; using the " + "highest-%s valid stored sample instead.", + quantity_name, + quantity_name, ) return instance @@ -440,6 +460,19 @@ def max_log_likelihood( "instance." ) from last_error + def _next_valid_instance(self, last_error: Exception) -> ModelInstance: + """ + Recovery hook for the ``to_instance(recover="next_valid")`` policy, + currently used only by ``max_log_posterior``: substitute the + highest-posterior stored sample the current model can still build. + """ + return self._instance_from_next_valid_sample( + exclude_sample=self.max_log_posterior_sample, + sample_value=lambda candidate: candidate.log_posterior, + quantity_name="posterior", + last_error=last_error, + ) + @property def max_log_posterior_sample(self) -> Sample: return self.sample_list[self.max_log_posterior_index] @@ -459,14 +492,14 @@ def max_log_posterior_index(self) -> int: return 0 return int(np.nanargmax(log_posterior_list)) - @to_instance + @to_instance(recover="next_valid") def max_log_posterior(self) -> ModelInstance: """ The parameters of the maximum log posterior sample of the `NonLinearSearch` returned as a model instance. """ return self.parameter_lists[self.max_log_posterior_index] - @to_instance + @to_instance() def from_sample_index(self, sample_index: int) -> ModelInstance: """ The parameters of an individual sample of the non-linear search, returned as a model instance. diff --git a/autofit/non_linear/samples/summary.py b/autofit/non_linear/samples/summary.py index cb6dcd5e1..aff859c3f 100644 --- a/autofit/non_linear/samples/summary.py +++ b/autofit/non_linear/samples/summary.py @@ -55,7 +55,7 @@ def max_log_likelihood_sample(self): def median_pdf_sample(self): return self._median_pdf_sample - @to_instance + @to_instance() def median_pdf(self, as_instance: bool = True) -> List[float]: """ The parameters of the maximum log likelihood sample of the `NonLinearSearch` returned as a model instance or diff --git a/autofit/non_linear/search/abstract_search.py b/autofit/non_linear/search/abstract_search.py index 92569c3c2..ec318cb87 100644 --- a/autofit/non_linear/search/abstract_search.py +++ b/autofit/non_linear/search/abstract_search.py @@ -758,7 +758,7 @@ def start_resume_fit(self, analysis: Analysis, model: AbstractPriorModel) -> Res if mode == 1: try: samples_summary.max_log_likelihood() - except exc.FitException as error: + except (exc.FitException, exc.SamplesException) as error: samples = self._test_mode_samples_after_rejected_fit( samples=samples, error=error, @@ -786,7 +786,7 @@ def start_resume_fit(self, analysis: Analysis, model: AbstractPriorModel) -> Res def _test_mode_samples_after_rejected_fit( self, samples: Samples, - error: exc.FitException, + error: Exception, ) -> Samples: """Build valid representative samples after a mode-1 rejected result. diff --git a/autofit/non_linear/search/updater.py b/autofit/non_linear/search/updater.py index b79ce4e9a..d4cbae337 100644 --- a/autofit/non_linear/search/updater.py +++ b/autofit/non_linear/search/updater.py @@ -210,7 +210,7 @@ def _save_samples( try: instance = samples_summary.instance - except exc.FitException: + except (exc.FitException, exc.SamplesException): return samples, samples_summary, None, samples self._paths.save_samples_summary(samples_summary=samples_summary) diff --git a/test_autofit/non_linear/samples/test_samples.py b/test_autofit/non_linear/samples/test_samples.py index 45033dc43..c003244c9 100644 --- a/test_autofit/non_linear/samples/test_samples.py +++ b/test_autofit/non_linear/samples/test_samples.py @@ -205,6 +205,95 @@ def __init__(self, value): samples.valid_sample_instance_pairs() +def test__instances__skips_historical_invalid_points(): + samples = _guarded_samples( + parameter_lists=[[0.1], [0.9]], + log_likelihood_list=[2.0, 1.0], + weight_list=[0.5, 0.5], + ) + + instances = samples.instances + + assert len(instances) == 1 + assert instances[0].value == 0.9 + + +def test__max_log_posterior__historical_invalid_best_uses_next_valid_instance(): + samples = _guarded_samples( + parameter_lists=[[0.1], [0.9]], + log_likelihood_list=[2.0, 1.0], + weight_list=[0.5, 0.5], + ) + + assert samples.max_log_posterior(as_instance=False) == [0.1] + assert samples.max_log_posterior().value == 0.9 + + +def test__max_log_posterior__all_invalid_fails_clearly(): + samples = _guarded_samples( + parameter_lists=[[0.1], [0.2]], + log_likelihood_list=[2.0, 1.0], + weight_list=[0.5, 0.5], + ) + + with pytest.raises( + af.exc.SamplesException, + match="None of the stored samples can be reconstructed", + ): + samples.max_log_posterior() + + +def test__from_sample_index__historical_invalid_raises_samples_exception(): + samples = _guarded_samples( + parameter_lists=[[0.1], [0.9]], + log_likelihood_list=[2.0, 1.0], + weight_list=[0.5, 0.5], + ) + + assert samples.from_sample_index(sample_index=0, as_instance=False) == [0.1] + assert samples.from_sample_index(sample_index=1).value == 0.9 + + with pytest.raises( + af.exc.SamplesException, + match="from_sample_index", + ): + samples.from_sample_index(sample_index=0) + + +def test__median_pdf__historical_invalid_raises_samples_exception(): + samples = _guarded_samples( + parameter_lists=[[0.1], [0.2]], + log_likelihood_list=[2.0, 1.0], + weight_list=[0.5, 0.5], + ) + + assert samples.median_pdf(as_instance=False)[0] < 0.5 + + with pytest.raises(af.exc.SamplesException, match="median_pdf"): + samples.median_pdf() + + +def test__to_instance__does_not_hide_programming_errors(): + class _RaisesUnexpectedly: + def __init__(self, value): + raise ValueError("real bug") + + model = af.Model(_RaisesUnexpectedly) + samples = af.SamplesPDF( + model=model, + sample_list=af.Sample.from_lists( + model=model, + parameter_lists=[[1.0]], + log_likelihood_list=[1.0], + log_prior_list=[0.0], + weight_list=[1.0], + ), + ) + + with pytest.raises(ValueError, match="real bug"): + samples.from_sample_index(sample_index=0) + + def test__draw_randomly_via_pdf__historical_invalid_draw_is_retried(monkeypatch): from autofit.non_linear.samples import pdf diff --git a/test_autofit/non_linear/search/test_abstract_search.py b/test_autofit/non_linear/search/test_abstract_search.py index 1dd42d239..46d3ca092 100644 --- a/test_autofit/non_linear/search/test_abstract_search.py +++ b/test_autofit/non_linear/search/test_abstract_search.py @@ -640,7 +640,9 @@ def test__test_mode_1__valid_factor_graph_children_keep_per_analysis_model( child.instance.galaxies.value == pytest.approx(0.9) for child in result ) - def test__normal_mode__fitexception_still_propagates(self, monkeypatch): + def test__normal_mode__reconstruction_failure_raises_samples_exception( + self, monkeypatch + ): monkeypatch.delenv("PYAUTO_TEST_MODE", raising=False) model, rejected_samples = _model_and_rejected_samples(_RejectsLowValue) @@ -649,9 +651,11 @@ def test__normal_mode__fitexception_still_propagates(self, monkeypatch): analysis=af.m.MockAnalysis(), ) - with pytest.raises(af.exc.FitException): + with pytest.raises(af.exc.SamplesException) as error: result.samples_summary.instance + assert isinstance(error.value.__cause__, af.exc.FitException) + def test__test_mode_1__non_fitexception_still_propagates(self, monkeypatch): monkeypatch.setenv("PYAUTO_TEST_MODE", "1") model, rejected_samples = _model_and_rejected_samples(