Refactor: consolidate duplicated critical-curve plot helper; remove stale plan doc#611
Merged
Merged
Conversation
The MatPlot/MatWrap/Visuals/Output removal this plan described is complete: no MatPlot*, Visuals*, MatWrap*, mat_plot_2d/visuals_2d references or mat_plot/wrap/visuals directories remain in autolens/. The plot modules are already the function-based design the plan targeted, so the plan document is obsolete. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RSRerFzzFttWNeZ5C7U4fx
_compute_critical_curve_lines was copy-pasted verbatim into both
autolens/imaging/plot/fit_imaging_plots.py and
autolens/interferometer/plot/fit_interferometer_plots.py, and the two copies
had drifted: the imaging copy narrowly caught (ModuleNotFoundError, ValueError)
and loudly logged anything else (guarding the PyAutoGalaxy abd7b717 / PyAutoFit
#1280 silent-fallback regression), while the interferometer copy still had a
bare 'except Exception' that silently swallowed every failure — the exact mode
that regression guard exists to prevent. Model code imported inconsistent
behaviour: interferometer/model/{visualizer,plotter} got the silent copy,
point/model/* got the hardened one.
Move the single canonical (hardened) definition to autolens/plot/plot_utils.py
— which already re-exports the underlying _critical_curves_from/_caustics_from
primitives — and re-export it from both fit-plot modules so every existing
'from ...fit_{imaging,interferometer}_plots import _compute_critical_curve_lines'
import keeps resolving. All call sites now share one hardened definition.
Behaviour-preserving except on the interferometer error path, which is
intentionally lifted from silent-swallow to the hardened logging behaviour.
_plot_source_plane is deliberately left duplicated: the two copies genuinely
differ (mask source, zoom_extent_scale), so consolidating it is not
behaviour-neutral.
Witness tests (imaging error-path silent/logged assertions) updated to patch
and observe the symbol at its new home; interferometer plot tests do not pin
the error path, so none break. Dead imports (_caustics_from, numpy_lines)
trimmed from both modules.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RSRerFzzFttWNeZ5C7U4fx
The previous commit placed _compute_critical_curve_lines in autolens/plot/plot_utils.py. Importing any autolens.plot.* submodule runs the autolens/plot/__init__.py aggregator, which imports the fit-plot modules — so having a fit-plot module import from autolens.plot.plot_utils created a circular import that broke 'import autolens' entirely (subplot_fit imported from a partially-initialised module). Relocate the canonical definition to autolens/lens/plot/critical_curves.py. The autolens.lens.plot and autolens.lens package __init__ files are empty, so importing it from the fit-plot modules triggers no aggregator and no cycle. Critical curves are a lensing concept, so this is also the more natural home. plot_utils.py is reverted to its original one-line re-export. tracer_plots.py's lazy import is repointed at the canonical home, and the witness test patches/ observes autolens.lens.plot.critical_curves. Verified: 'import autolens' (incl. the autolens.plot aggregator) succeeds, and the imaging/interferometer fit-plot, tracer-plot, and imaging/interferometer/ point model test suites all pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RSRerFzzFttWNeZ5C7U4fx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A behaviour-preserving refactor of the fit-plot subsystem, plus removal of a completed refactor's stale planning doc.
1. Remove
PLOT_REFACTOR_PLAN.mdThe
MatPlot/MatWrap/Visuals/Outputremoval this document planned is fully shipped — noMatPlot*,Visuals*,MatWrap*,mat_plot_2d/visuals_2dreferences and nomat_plot/wrap/visualsdirectories remain anywhere inautolens/. The plot modules are already the function-based design the plan targeted, so the document is obsolete.2. Consolidate the duplicated
_compute_critical_curve_lineshelper_compute_critical_curve_lineswas copy-pasted verbatim into bothautolens/imaging/plot/fit_imaging_plots.pyandautolens/interferometer/plot/fit_interferometer_plots.py, and the two copies had drifted:(ModuleNotFoundError, ValueError)and loudly logged anything else — a guard added for a real regression (ZeroSolverraising inside a model-fit and viz silently falling back to all-zero overlays);except Exceptionthat silently swallowed every failure — the exact mode that guard exists to prevent.Model code even imported inconsistent behaviour depending on the path:
interferometer/model/{visualizer,plotter}got the silent copy whilepoint/model/*got the hardened one.This moves the single canonical (hardened) definition to
autolens/lens/plot/critical_curves.pyand re-exports it from both fit-plot modules, so every existingimportkeeps resolving — no call sites changed. Dead imports (_caustics_from,numpy_lines) were trimmed from both modules.Behaviour: preserved everywhere except the interferometer error path, which is intentionally lifted from silent-swallow to the hardened logging behaviour (closing a latent silent-failure bug the duplication had been hiding).
Deliberately left alone:
_plot_source_planeis also duplicated across the two modules, but the copies genuinely differ (fit.maskvsfit.dataset.real_space_mask; imaging's extrazoom_extent_scalelogic), so consolidating it would not be behaviour-neutral. Out of scope for a behaviour-preserving refactor.Why
autolens/lens/plot/and notautolens/plot/Importing any
autolens.plot.*submodule runs theautolens/plot/__init__.pyaggregator, which imports the fit-plot modules — so homing the helper there creates a circular import. Theautolens.lens.plot/autolens.lenspackage__init__files are empty, so importing the helper from the fit-plot modules triggers no aggregator and no cycle. Critical curves are a lensing concept, so this is also the more natural home.Testing
Installed the
autogalaxy 2026.7.15.1release stack (matching API) in a venv and ran:import autolens(including theautolens.plotaggregator that the intermediate cycle broke) succeeds; all three names resolve to one shared object atautolens.lens.plot.critical_curves.test_fit_imaging_plots.py+test_fit_interferometer_plots.py→ 17 passed;test_tracer_plots.py+ imaging/interferometer/pointmodelsuites → 28 passed, 3 skipped; full plot test subtrees → 27 passed.The imaging error-path witness tests (silent-vs-logged assertions) were updated to patch/observe the helper at its new home; the interferometer plot tests do not pin the error path, so none break.
🤖 Generated with Claude Code
Generated by Claude Code