From e2c45e19f6eb549a5cd27504c168378351200d8d Mon Sep 17 00:00:00 2001 From: James Nightingale Date: Tue, 11 Aug 2026 12:41:02 -0400 Subject: [PATCH] fix: preserve guarded sample lifecycle --- autofit/aggregator/base.py | 68 ++++++++--- autofit/non_linear/samples/pdf.py | 50 +++++++- autofit/non_linear/samples/samples.py | 60 +++++++++- autofit/non_linear/search/abstract_search.py | 10 +- test_autofit/aggregator/test_base.py | 111 ++++++++++++++++++ .../non_linear/samples/test_samples.py | 66 +++++++++++ .../non_linear/search/test_abstract_search.py | 48 ++++++++ 7 files changed, 380 insertions(+), 33 deletions(-) create mode 100644 test_autofit/aggregator/test_base.py diff --git a/autofit/aggregator/base.py b/autofit/aggregator/base.py index a1214ca97..164ed2681 100644 --- a/autofit/aggregator/base.py +++ b/autofit/aggregator/base.py @@ -1,10 +1,13 @@ from __future__ import annotations from abc import ABC, abstractmethod from functools import partial +import logging from typing import List, Optional, Generator import autofit as af +logger = logging.getLogger(__name__) + class AggBase(ABC): def __init__(self, aggregator: af.Aggregator): @@ -82,13 +85,13 @@ def weights_above_gen_from(self, minimum_weight: float) -> List: def func_gen(fit: af.Fit, minimum_weight: float) -> List[object]: samples = fit.samples - weight_list = [] - - for sample in samples.sample_list: - if sample.weight > minimum_weight: - weight_list.append(sample.weight) - - return weight_list + return [ + sample.weight + for sample, _ in self._valid_sample_instance_pairs( + samples=samples, + minimum_weight=minimum_weight, + ) + ] func = partial(func_gen, minimum_weight=minimum_weight) @@ -119,22 +122,51 @@ def all_above_weight_gen_from(self, minimum_weight: float) -> Generator: def func_gen(fit: af.Fit, minimum_weight: float) -> List[object]: samples = fit.samples - all_above_weight_list = [] - - for sample in samples.sample_list: - if sample.weight > minimum_weight: - instance = sample.instance_for_model(model=samples.model) - - all_above_weight_list.append( - self.object_via_gen_from(fit=fit, instance=instance) - ) - - return all_above_weight_list + return [ + self.object_via_gen_from(fit=fit, instance=instance) + for _, instance in self._valid_sample_instance_pairs( + samples=samples, + minimum_weight=minimum_weight, + ) + ] func = partial(func_gen, minimum_weight=minimum_weight) return self.aggregator.map(func=func) + @staticmethod + def _valid_sample_instance_pairs(samples, minimum_weight: float): + """Return weighted samples whose model instances still reconstruct. + + Constructor validation can become stricter after a result was written. + Such historical points are not usable objects, but they must not make an + entire aggregator query fail. ``FitException`` is the narrow model-point + rejection contract; programming errors continue to propagate. + """ + pairs = [] + rejected = 0 + + for sample in samples.sample_list: + if sample.weight <= minimum_weight: + continue + try: + instance = samples.model.instance_from_vector( + sample.parameter_lists_for_model(model=samples.model) + ) + except af.exc.FitException: + rejected += 1 + continue + pairs.append((sample, instance)) + + if rejected: + logger.warning( + "Skipped %d stored sample(s) rejected by current model " + "validation while building aggregator objects.", + rejected, + ) + + return pairs + def randomly_drawn_via_pdf_gen_from(self, total_samples: int): """ Returns a generator which for every result generates a list of objects whose parameter values are drawn diff --git a/autofit/non_linear/samples/pdf.py b/autofit/non_linear/samples/pdf.py index 5ff9b11a1..d7c99b2b5 100644 --- a/autofit/non_linear/samples/pdf.py +++ b/autofit/non_linear/samples/pdf.py @@ -1,4 +1,5 @@ import math +import logging import pathlib import warnings from typing import Dict, List, Optional, Tuple, Union @@ -6,6 +7,7 @@ import numpy as np from autonerves import conf +from autofit import exc from autonerves.output import should_output from autofit.mapper.model import ModelInstance from autofit.mapper.prior_model.abstract import AbstractPriorModel @@ -14,6 +16,10 @@ from .samples import Samples from .summary import SamplesSummary +logger = logging.getLogger(__name__) + +VALID_INSTANCE_MAX_ATTEMPTS = 100 + class SamplesPDF(Samples): def __init__( @@ -312,8 +318,11 @@ def error_magnitudes_at_sigma(self, sigma: float) -> Union[List, ModelInstance]: lowers = self.values_at_lower_sigma(sigma=sigma, as_instance=False) return list(map(lambda upper, lower: upper - lower, uppers, lowers)) - @to_instance - def draw_randomly_via_pdf(self) -> Union[List, ModelInstance]: + def draw_randomly_via_pdf( + self, + as_instance: bool = True, + as_dict: bool = False, + ) -> Union[List, Dict, ModelInstance]: """ The parameter vector of an individual sample of the non-linear search drawn randomly from the PDF, returned as a 1D list. @@ -322,11 +331,40 @@ def draw_randomly_via_pdf(self) -> Union[List, ModelInstance]: for non-linear searches like nested sampling). """ - sample_index = np.random.choice( - a=range(len(self.sample_list)), p=self.weight_list - ) + last_error = None - return self.parameter_lists[sample_index][:] + for attempt in range(VALID_INSTANCE_MAX_ATTEMPTS): + sample_index = np.random.choice( + a=range(len(self.sample_list)), p=self.weight_list + ) + vector = self.parameter_lists[sample_index][:] + + if as_dict: + return { + ".".join(path[0]): value for path, value in zip(self.paths, vector) + } + + if not as_instance: + return vector + + try: + instance = self._instance_from_vector(vector) + except exc.FitException as error: + last_error = error + continue + + if attempt > 0: + logger.warning( + "A randomly drawn stored sample can no longer be " + "reconstructed because the model rejected it with " + "FitException; drew another stored sample instead." + ) + return instance + + raise exc.SamplesException( + "Could not draw a valid model instance from the stored PDF after " + f"{VALID_INSTANCE_MAX_ATTEMPTS} attempts." + ) from last_error def samples_drawn_randomly_via_pdf_from(self, total_draws: int = 100) -> "SamplesPDF": """ diff --git a/autofit/non_linear/samples/samples.py b/autofit/non_linear/samples/samples.py index 0de283e63..cda092f27 100644 --- a/autofit/non_linear/samples/samples.py +++ b/autofit/non_linear/samples/samples.py @@ -322,19 +322,71 @@ def max_log_likelihood_index(self) -> int: return 0 return int(np.nanargmax(log_likelihood_list)) - @to_instance - def max_log_likelihood(self) -> List[float]: + def max_log_likelihood( + self, + as_instance: bool = True, + as_dict: bool = False, + ) -> Union[List[float], Dict, ModelInstance]: """ The parameters of the maximum log likelihood sample of the `NonLinearSearch` returned as a model instance or list of values. + + When an older stored result contains a point which a newer model class + rejects with :class:`FitException`, instance reconstruction falls back + to the next-highest-likelihood valid point. The recorded best vector is + still returned unchanged when ``as_instance=False`` or ``as_dict=True``; + only the request to materialize an object needs this compatibility path. """ sample = self.max_log_likelihood_sample - - return sample.parameter_lists_for_paths( + vector = sample.parameter_lists_for_paths( self.paths if sample.is_path_kwargs else self.names ) + if as_dict: + return {".".join(path[0]): value for path, value in zip(self.paths, vector)} + + if not as_instance: + return vector + + try: + return self._instance_from_vector(vector) + except exc.FitException as error: + last_error = error + + valid_sample_candidates = sorted( + (candidate for candidate in self.sample_list if candidate is not sample), + key=lambda candidate: ( + float("-inf") + if np.isnan(candidate.log_likelihood) + else candidate.log_likelihood + ), + reverse=True, + ) + + for candidate in valid_sample_candidates: + candidate_vector = candidate.parameter_lists_for_paths( + self.paths if candidate.is_path_kwargs else self.names + ) + try: + instance = self._instance_from_vector(candidate_vector) + except exc.FitException as error: + last_error = error + 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." + ) + return instance + + raise exc.SamplesException( + "None of the stored samples can be reconstructed as a valid model " + "instance." + ) from last_error + @property def max_log_posterior_sample(self) -> Sample: return self.sample_list[self.max_log_posterior_index] diff --git a/autofit/non_linear/search/abstract_search.py b/autofit/non_linear/search/abstract_search.py index e529e13d9..8b1fcf29e 100644 --- a/autofit/non_linear/search/abstract_search.py +++ b/autofit/non_linear/search/abstract_search.py @@ -759,7 +759,7 @@ def start_resume_fit(self, analysis: Analysis, model: AbstractPriorModel) -> Res samples_summary.instance except exc.FitException as error: samples = self._test_mode_samples_after_rejected_fit( - model=model, + samples=samples, error=error, ) samples_summary = samples.summary() @@ -784,7 +784,7 @@ def start_resume_fit(self, analysis: Analysis, model: AbstractPriorModel) -> Res def _test_mode_samples_after_rejected_fit( self, - model: AbstractPriorModel, + samples: Samples, error: exc.FitException, ) -> Samples: """Build valid representative samples after a mode-1 rejected result. @@ -801,8 +801,6 @@ def _test_mode_samples_after_rejected_fit( rejected point. The fixed seed keeps smoke tests reproducible without changing the application's global random state. """ - from autofit.non_linear.samples.pdf import SamplesPDF - logger.warning( "TEST MODE 1: the reduced search's final sample raised " f"FitException ({error.__cause__ or error!r}); replacing it with " @@ -811,6 +809,7 @@ def _test_mode_samples_after_rejected_fit( rng = np.random.default_rng(seed=0) last_error = error + model = samples.model for attempt in range(TEST_MODE_REPRESENTATIVE_MAX_ATTEMPTS): unit_vector = ( @@ -841,13 +840,14 @@ def _test_mode_samples_after_rejected_fit( continue samples_info = { + **(samples.samples_info or {}), "total_iterations": 1, "time": 0.0, "log_evidence": -1.0e99, } samples_info.update(self._test_mode_samples_info()) - return SamplesPDF( + return samples.from_list_info_and_model( model=model, sample_list=sample_list, samples_info=samples_info, diff --git a/test_autofit/aggregator/test_base.py b/test_autofit/aggregator/test_base.py new file mode 100644 index 000000000..37cc9cbb4 --- /dev/null +++ b/test_autofit/aggregator/test_base.py @@ -0,0 +1,111 @@ +import pytest + +import autofit as af + +from autofit.aggregator.base import AggBase + + +class _RejectsLowStoredValue: + def __init__(self, value): + if value < 0.5: + raise af.exc.FitException("stored value is outside the current domain") + self.value = value + + +class _Fit: + def __init__(self, samples): + self.samples = samples + + +class _Aggregator: + def __init__(self, fit): + self.fit = fit + + def map(self, func): + return [func(self.fit)] + + +class _InstanceAgg(AggBase): + def object_via_gen_from(self, fit, instance=None): + return instance + + +def _samples_with_historical_invalid_point(): + model = af.Model(_RejectsLowStoredValue) + model.value = af.UniformPrior(lower_limit=0.0, upper_limit=1.0) + return af.SamplesPDF( + model=model, + sample_list=af.Sample.from_lists( + model=model, + parameter_lists=[[0.1], [0.9]], + log_likelihood_list=[2.0, 1.0], + log_prior_list=[0.0, 0.0], + weight_list=[0.6, 0.4], + ), + ) + + +def test__weighted_aggregator_objects_skip_invalid_historical_samples(): + samples = _samples_with_historical_invalid_point() + agg = _InstanceAgg(aggregator=_Aggregator(fit=_Fit(samples=samples))) + + objects = agg.all_above_weight_gen_from(minimum_weight=-1.0) + weights = agg.weights_above_gen_from(minimum_weight=-1.0) + + assert [[instance.value for instance in result] for result in objects] == [[0.9]] + assert weights == [[0.4]] + + +def test__weighted_aggregator_preserves_shared_factor_graph_children(): + shared_value = af.UniformPrior(lower_limit=0.0, upper_limit=1.0) + factor_graph = af.FactorGraphModel( + *[ + af.AnalysisFactor( + prior_model=af.Collection( + galaxies=af.Model(_RejectsLowStoredValue, value=shared_value) + ), + analysis=af.m.MockAnalysis(), + ) + for _ in range(2) + ] + ) + model = factor_graph.global_prior_model + samples = af.SamplesPDF( + model=model, + sample_list=af.Sample.from_lists( + model=model, + parameter_lists=[[0.9]], + log_likelihood_list=[1.0], + log_prior_list=[0.0], + weight_list=[1.0], + ), + ) + agg = _InstanceAgg(aggregator=_Aggregator(fit=_Fit(samples=samples))) + + objects = agg.all_above_weight_gen_from(minimum_weight=-1.0) + + instance = objects[0][0] + assert getattr(instance, "0").galaxies.value == 0.9 + assert getattr(instance, "1").galaxies.value == 0.9 + + +def test__weighted_aggregator_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], + ), + ) + agg = _InstanceAgg(aggregator=_Aggregator(fit=_Fit(samples=samples))) + + with pytest.raises(ValueError, match="real bug"): + agg.all_above_weight_gen_from(minimum_weight=-1.0) diff --git a/test_autofit/non_linear/samples/test_samples.py b/test_autofit/non_linear/samples/test_samples.py index 1a3cb31b3..f3e43cff2 100644 --- a/test_autofit/non_linear/samples/test_samples.py +++ b/test_autofit/non_linear/samples/test_samples.py @@ -7,6 +7,28 @@ pytestmark = pytest.mark.filterwarnings("ignore::FutureWarning") +class _RejectsLowStoredValue: + def __init__(self, value): + if value < 0.5: + raise af.exc.FitException("stored value is outside the current domain") + self.value = value + + +def _guarded_samples(parameter_lists, log_likelihood_list, weight_list): + model = af.Model(_RejectsLowStoredValue) + model.value = af.UniformPrior(lower_limit=0.0, upper_limit=1.0) + return af.SamplesPDF( + model=model, + sample_list=af.Sample.from_lists( + model=model, + parameter_lists=parameter_lists, + log_likelihood_list=log_likelihood_list, + log_prior_list=[0.0] * len(parameter_lists), + weight_list=weight_list, + ), + ) + + def test__table__headers(samples_x5): assert samples_x5._headers == [ "mock_class_1.one", @@ -71,6 +93,48 @@ def test__max_log_likelihood(samples_x5): assert instance.mock_class_1.four == 24.0 +def test__max_log_likelihood__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_likelihood(as_instance=False) == [0.1] + assert samples.max_log_likelihood().value == 0.9 + + +def test__draw_randomly_via_pdf__historical_invalid_draw_is_retried(monkeypatch): + from autofit.non_linear.samples import pdf + + samples = _guarded_samples( + parameter_lists=[[0.1], [0.9]], + log_likelihood_list=[2.0, 1.0], + weight_list=[0.5, 0.5], + ) + choices = iter([0, 1]) + monkeypatch.setattr(pdf.np.random, "choice", lambda *args, **kwargs: next(choices)) + + assert samples.draw_randomly_via_pdf().value == 0.9 + + +def test__draw_randomly_via_pdf__all_invalid_fails_clearly(monkeypatch): + from autofit.non_linear.samples import pdf + + samples = _guarded_samples( + parameter_lists=[[0.1]], + log_likelihood_list=[1.0], + weight_list=[1.0], + ) + monkeypatch.setattr(pdf, "VALID_INSTANCE_MAX_ATTEMPTS", 2) + + with pytest.raises( + af.exc.SamplesException, + match="Could not draw a valid model instance.*after 2 attempts", + ): + samples.draw_randomly_via_pdf() + + def test__max_log_posterior(): model = af.Collection(mock_class_1=af.m.MockClassx4) @@ -165,6 +229,7 @@ def test__samples_above_weight_threshold_from(): assert len(samples_above_weight_threshold) == 3 assert samples_above_weight_threshold.sample_list[0].weight == 1.0 + def test__samples_drawn_randomly_via_pdf_from(): model = af.Collection(mock_class=af.m.MockClassx4) @@ -195,6 +260,7 @@ def test__samples_drawn_randomly_via_pdf_from(): assert len(samples_drawn_randomly_via_pdf) == 3 assert samples_drawn_randomly_via_pdf.sample_list[0].weight == 0.2 + def test__addition_of_samples(samples_x5): samples = samples_x5 + samples_x5 diff --git a/test_autofit/non_linear/search/test_abstract_search.py b/test_autofit/non_linear/search/test_abstract_search.py index 7ef8c1840..3e4765662 100644 --- a/test_autofit/non_linear/search/test_abstract_search.py +++ b/test_autofit/non_linear/search/test_abstract_search.py @@ -516,6 +516,12 @@ def perform_update( return self._rejected_samples +class _TaggedSamplesPDF(af.SamplesPDF): + """Proves test-mode recovery retains the sampler's concrete sample type.""" + + pass + + def _model_and_rejected_samples(cls): model = af.Model(cls) model.value = af.UniformPrior(lower_limit=0.0, upper_limit=1.0) @@ -548,6 +554,48 @@ def test__test_mode_1__fitexception_gets_valid_representative(self, monkeypatch) ) assert all(instance.value >= 0.75 for instance in result.samples.instances) + def test__test_mode_1__factor_graph_children_and_sample_type_are_preserved( + self, monkeypatch + ): + monkeypatch.setenv("PYAUTO_TEST_MODE", "1") + + shared_value = af.UniformPrior(lower_limit=0.0, upper_limit=1.0) + factor_models = [ + af.Collection(galaxies=af.Model(_RejectsLowValue, value=shared_value)) + for _ in range(2) + ] + factor_graph = af.FactorGraphModel( + *[ + af.AnalysisFactor( + prior_model=factor_model, + analysis=af.m.MockAnalysis(), + ) + for factor_model in factor_models + ] + ) + model = factor_graph.global_prior_model + rejected_samples = _TaggedSamplesPDF( + model=model, + sample_list=af.Sample.from_lists( + model=model, + parameter_lists=[[0.1]], + log_likelihood_list=[-1.0e99], + log_prior_list=[0.0], + weight_list=[1.0], + ), + samples_info={"log_evidence": -1.0e99, "sampler_marker": "retained"}, + ) + + result = _RejectedFinalSampleSearch(samples=rejected_samples).fit( + model=model, + analysis=factor_graph, + ) + + assert type(result.samples) is _TaggedSamplesPDF + assert result.samples.samples_info["sampler_marker"] == "retained" + assert len(result) == 2 + assert all(child.instance.galaxies.value >= 0.75 for child in result) + def test__normal_mode__fitexception_still_propagates(self, monkeypatch): monkeypatch.delenv("PYAUTO_TEST_MODE", raising=False) model, rejected_samples = _model_and_rejected_samples(_RejectsLowValue)