From 1d8ae08b96b91ff284204a4750bfca269af676d0 Mon Sep 17 00:00:00 2001 From: Jammy2211 Date: Fri, 28 Aug 2026 10:00:52 -0400 Subject: [PATCH] feat: declare the ell_comps disk on EllProfile so a search can project onto it (PyAutoFit#1537) `EllProfile.__model_constraint__` already MEASURES how far outside the ellipticity clamp a profile sits, but a measure cannot say how to fix one. This declares the structure PyAutoFit's new `ClipperPriorBoxJoint` projects onto: __model_ball_constraints__ = ((("ell_comps",), convert.ELL_COMPS_MAGNITUDE_CLAMP),) The radius is the CLAMP (0.999), deliberately not `1 - margin`. Between 0.999 and 1.0 the conversion to an axis ratio saturates, so the likelihood is flat radially and a lane projected into that annulus has nothing to climb back out on -- it would be moved from a region the model rejects into one the optimizer cannot leave. Projecting onto the clamp puts it exactly where the radial gradient is alive again. Declared once at `EllProfile`, so it reaches every elliptical light and mass profile; the spherical subclasses inherit it but pin `ell_comps` to an instance, so PyAutoFit resolves no pair and projects nothing. `validate_ell_comps` is deliberately unchanged. Making the guard fire on the traced path would turn a 20%-of-lanes condition into a 20%-of-lanes crash in the middle of a multi-hour fit -- the fix belongs in the search's constraint handling, not in a validator that converts a survivable state into an exception. Also widens `AnalysisDataset.save_results`' catch to `(AttributeError, af.exc.SamplesException, af.exc.FitException)`, mirroring PyAutoLens#713: building the galaxies materializes the maximum log likelihood sample as a model instance, which the model may reject, and writing an optional output file must never kill a completed fit before `paths.completed()` (PyAutoFit#1535). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_011joies4k5TdRqezPUK8YET --- autogalaxy/analysis/analysis/dataset.py | 12 +- autogalaxy/profiles/geometry_profiles.py | 15 ++ .../analysis/test_analysis_dataset.py | 40 ++++++ .../profiles/test_model_constraint.py | 131 ++++++++++++++++++ 4 files changed, 196 insertions(+), 2 deletions(-) diff --git a/autogalaxy/analysis/analysis/dataset.py b/autogalaxy/analysis/analysis/dataset.py index 3f1e52db..0631ab52 100644 --- a/autogalaxy/analysis/analysis/dataset.py +++ b/autogalaxy/analysis/analysis/dataset.py @@ -175,8 +175,16 @@ def save_results(self, paths: af.DirectoryPaths, result: ResultDataset): obj=result.max_log_likelihood_galaxies, file_path=paths._files_path / "galaxies.json", ) - except AttributeError: - pass + except (AttributeError, af.exc.SamplesException, af.exc.FitException) as e: + # Building the galaxies requires materializing the maximum log likelihood + # sample as a model instance, which the model may reject (e.g. `ell_comps` + # outside the unit disk). Writing an extra output file must never kill a + # completed fit before `paths.completed()` is called (PyAutoFit #1535), so + # the failure is logged and the fit finishes without `galaxies.json`. + logger.warning( + f"The maximum log likelihood galaxies could not be written to " + f"galaxies.json, the model-fit is otherwise unaffected:\n{e}" + ) def adapt_images_via_instance_from( self, diff --git a/autogalaxy/profiles/geometry_profiles.py b/autogalaxy/profiles/geometry_profiles.py index 4d2bdd1d..ba63f87f 100644 --- a/autogalaxy/profiles/geometry_profiles.py +++ b/autogalaxy/profiles/geometry_profiles.py @@ -237,6 +237,21 @@ def __init__( validate.validate_ell_comps(ell_comps=ell_comps) self.ell_comps = ell_comps + # The projectable form of the constraint below, read by PyAutoFit's + # `ClipperPriorBoxJoint` (see `autofit.mapper.prior_model.constraint`). + # `__model_constraint__` MEASURES how far outside the disk a profile sits; + # this states the disk itself, which is what a search needs to put a lane + # back inside it. + # + # The radius is the CLAMP (0.999), not the guard's 1.0 and not `1 - margin` + # for any small margin. Between 0.999 and 1.0 the conversion to an axis + # ratio saturates, so the likelihood is flat in the radial direction and a + # gradient lane projected into that annulus has nothing to climb back out + # on -- it would be moved from a region the model rejects into one the + # optimizer cannot leave. Projecting onto the clamp puts the lane exactly at + # the edge of the region where the radial gradient is alive again. + __model_ball_constraints__ = ((("ell_comps",), convert.ELL_COMPS_MAGNITUDE_CLAMP),) + def __model_constraint__(self, xp=np): """ How far beyond the ellipticity clamp this profile's `ell_comps` sit. diff --git a/test_autogalaxy/analysis/analysis/test_analysis_dataset.py b/test_autogalaxy/analysis/analysis/test_analysis_dataset.py index c0cbb8a7..733ed892 100644 --- a/test_autogalaxy/analysis/analysis/test_analysis_dataset.py +++ b/test_autogalaxy/analysis/analysis/test_analysis_dataset.py @@ -80,3 +80,43 @@ def test__instance_with_associated_adapt_images_from__galaxy_name_image_plane_me assert adapt_images.galaxy_image_plane_mesh_grid_dict[ galaxies.source ].native == pytest.approx(4.0 * np.ones((2, 2)), 1.0e-4) + + +class _RaisingGalaxiesResult: + """ + Result double whose galaxies cannot be built, because materializing the maximum log + likelihood sample as a model instance fails. + """ + + def __init__(self, error): + self._error = error + + @property + def max_log_likelihood_galaxies(self): + raise self._error + + +@pytest.mark.parametrize( + "error", + [ + AttributeError("no galaxies on this result"), + af.exc.SamplesException("stored parameters cannot be reconstructed"), + af.exc.FitException("ell_comps must satisfy e0**2+e1**2 < 1"), + ], +) +def test__save_results__galaxies_failure_never_kills_the_fit( + analysis_imaging_7x7, error +): + """ + `save_results` runs after the search has finished but before `paths.completed()`, so a + failure writing the (optional) `galaxies.json` must be logged and swallowed rather than + losing the run its `.completed` marker (PyAutoFit #1535). + """ + paths = af.DirectoryPaths() + + analysis_imaging_7x7.save_results( + paths=paths, + result=_RaisingGalaxiesResult(error), + ) + + assert not (paths._files_path / "galaxies.json").exists() diff --git a/test_autogalaxy/profiles/test_model_constraint.py b/test_autogalaxy/profiles/test_model_constraint.py index ce73d53d..7273c7c7 100644 --- a/test_autogalaxy/profiles/test_model_constraint.py +++ b/test_autogalaxy/profiles/test_model_constraint.py @@ -1,8 +1,12 @@ +import inspect + import numpy as np import pytest +import autofit as af import autogalaxy as ag from autogalaxy import convert +from autogalaxy.profiles import validate from autogalaxy.profiles.geometry_profiles import EllProfile @@ -83,3 +87,130 @@ def test_conversion_saturates_at_the_constant(self): def test_value_is_unchanged_by_the_refactor(self): assert convert.ELL_COMPS_MAGNITUDE_CLAMP == 0.999 + + +def _ell_profile_subclasses(): + """Every `EllProfile` subclass reachable from the public `ag.lp` / `ag.mp` + namespaces, so a profile added later is covered without editing this list.""" + classes = set() + for namespace in (ag.lp, ag.mp, ag.lmp): + for _, obj in inspect.getmembers(namespace, inspect.isclass): + if issubclass(obj, EllProfile): + classes.add(obj) + return sorted(classes, key=lambda cls: cls.__name__) + + +class TestBallDeclaration: + def test_there_are_profiles_to_check(self): + """Guards the sweep below: an empty namespace scan would pass vacuously.""" + assert len(_ell_profile_subclasses()) > 20 + + def test_every_elliptical_profile_declares_the_ball(self): + """`ell_comps` has one assignment site, at `EllProfile`, so the ball + declaration reaches every elliptical light and mass profile — including + the spherical ones, whose components are pinned and therefore never + projected.""" + for cls in _ell_profile_subclasses(): + assert cls.__model_ball_constraints__ == ( + (("ell_comps",), convert.ELL_COMPS_MAGNITUDE_CLAMP), + ), cls.__name__ + + def test_the_radius_is_the_clamp_not_the_validity_boundary(self): + """Between 0.999 and 1.0 the conversion to an axis ratio saturates, so the + likelihood is flat radially. Projecting onto `1 - margin` would move a + lane from a region the model rejects into one the optimizer cannot leave; + projecting onto the clamp puts it where the gradient is alive again.""" + ((_, radius),) = EllProfile.__model_ball_constraints__ + + assert radius == convert.ELL_COMPS_MAGNITUDE_CLAMP + assert radius == 0.999 + assert radius < 1.0 + + def test_pyautofit_resolves_the_declaration_to_a_parameter_pair(self): + """The declaration is only useful if PyAutoFit can turn it into indices + into the vector a search steps.""" + model = af.Collection( + galaxies=af.Collection( + lens=af.Model(ag.Galaxy, redshift=0.5, mass=ag.mp.Isothermal), + ) + ) + + ((index_0, index_1, radius),) = model.ball_constraint_index_pairs() + + names = [tuple_.name for tuple_ in model.prior_tuples_ordered_by_id] + assert names[index_0] == "ell_comps_0" + assert names[index_1] == "ell_comps_1" + assert radius == convert.ELL_COMPS_MAGNITUDE_CLAMP + + def test_a_spherical_profile_contributes_no_pair(self): + """`IsothermalSph` inherits the declaration but pins `ell_comps` to + `(0, 0)`, so there is no free pair to project.""" + model = af.Collection( + galaxies=af.Collection( + lens=af.Model(ag.Galaxy, redshift=0.5, mass=ag.mp.IsothermalSph), + ) + ) + + assert model.ball_constraint_index_pairs() == () + + def test_the_joint_clipper_projects_a_real_lens_model(self): + """End to end, with a real profile and PyAutoFit's opt-in clipper: a lane + at `|e| = 1.4` — inside both `ell_comps` prior boxes, outside the disk — + comes back inside it.""" + model = af.Collection( + galaxies=af.Collection( + lens=af.Model(ag.Galaxy, redshift=0.5, mass=ag.mp.Isothermal), + ) + ) + + ((index_0, index_1, radius),) = model.ball_constraint_index_pairs() + + vector = np.array(model.physical_values_from_prior_medians) + vector[index_0] = 1.4 / np.sqrt(2.0) + vector[index_1] = 1.4 / np.sqrt(2.0) + + projected, mask = af.ClipperPriorBoxJoint(margin=0.0).project( + vector=vector, model=model + ) + + assert np.hypot(projected[index_0], projected[index_1]) == pytest.approx(radius) + assert mask[index_0] + assert mask[index_1] + + +class TestGuardIsUntouched: + """The ball is a *search-side* projection. `validate_ell_comps`'s + standalone-construction behaviour is deliberately unchanged: making it fire on + the traced path would turn a 20%-of-lanes condition into a 20%-of-lanes crash + in the middle of a multi-hour fit.""" + + def test_it_still_rejects_a_magnitude_of_one_on_construction(self): + with pytest.raises(Exception): + ag.mp.Isothermal(ell_comps=(1.2, 0.0)) + + def test_it_still_rejects_the_corner_the_box_permits(self): + with pytest.raises(Exception): + ag.mp.Isothermal(ell_comps=(0.8, 0.8)) + + def test_it_still_accepts_the_saturating_annulus(self): + """0.999 <= magnitude < 1.0 remains constructible. The constraint flags it + and the clipper projects out of it; the guard does not raise on it, and + that has not changed.""" + assert ag.mp.Isothermal(ell_comps=(0.9995, 0.0)) is not None + + def test_it_still_returns_early_for_a_non_concrete_magnitude(self): + """The escape hatch that makes the guard a no-op under a trace, which is + why the search needed a projection in the first place.""" + + class Tracer: + def __mul__(self, other): + return self + + __rmul__ = __mul__ + __add__ = __mul__ + __radd__ = __mul__ + + def __float__(self): + raise TypeError("tracer") + + validate.validate_ell_comps(ell_comps=(Tracer(), Tracer()))