fix: preserve guarded sample lifecycle (#1466)

Author: Jammy2211

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

autofit/non_linear/samples/pdf.py

@@ -1,11 +1,13 @@
 import math
+import logging
 import pathlib
 import warnings
 from typing import Dict, List, Optional, Tuple, Union
 
 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":
         """

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]

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,