Skip to content

fix: correct unphysical ell_comps in tutorial 3 model-fitting exercise - #65

Merged
Jammy2211 merged 1 commit into
mainfrom
claude/ell-comps-validation-guard-kqpxry
Aug 13, 2026
Merged

fix: correct unphysical ell_comps in tutorial 3 model-fitting exercise#65
Jammy2211 merged 1 commit into
mainfrom
claude/ell-comps-validation-guard-kqpxry

Conversation

@Jammy2211

Copy link
Copy Markdown
Collaborator

Summary

The manual model-fitting exercise at the end of tutorial_3_fitting shipped ell_comps=(0.5, 0.9) as its deliberately-poor starting guess. That pair has magnitude f = 1.0296, and the axis ratio is q = (1 - f) / (1 + f), so it implies q = -0.015 — an ellipse with a negative axis ratio, which has no geometric meaning.

The value was always unphysical. It only became visible when PyAutoGalaxy added the validate_ell_comps guard on EllProfile (the single base every elliptical profile inherits), which rejects f >= 1.

Changed to (0.5, 0.5): f = 0.707, q = 0.17. Still a wildly wrong starting point against the simulator's true bulge (axis_ratio=0.9, angle=45deg, i.e. ell_comps ~ (0.053, 0.0)), so the exercise keeps its "adjust these to improve the fit" pedagogy — the neighbouring centre=(1.0, 10) is left deliberately bad too. The profile is now geometrically valid.

Why fix the value rather than the guard

The guard is correct and correctly placed: q = (1 - f) / (1 + f) is a valid axis ratio in (0, 1] only while f < 1; at f == 1 the ellipse degenerates to q == 0 and beyond it q goes negative. Relaxing it would re-admit profiles that return a finite but meaningless image.

Scope

Applied to the script and to both generated mirrors, so the run_scripts and run_notebooks legs agree:

  • scripts/chapter_1_introduction/tutorial_3_fitting.py
  • notebooks/chapter_1_introduction/tutorial_3_fitting.ipynb
  • markdown/chapter_1_introduction/tutorial_3_fitting.md

Evidence

Surfaced by PyAutoHeart Workspace Smoke run 31356506626 (2026-08-10). The howtogalaxy / chapter_1_introduction leg had exactly one failure — this script — with the other six passing:

scripts/chapter_1_introduction/tutorial_3_fitting.py ...   FAIL (19.0s)
ValueError: ell_comps must satisfy ell_comps[0]**2 + ell_comps[1]**2 < 1;
got (0.5, 0.9), whose magnitude is np.float64(1.0295630140987)

An AST scan of 454 ell_comps literals across autogalaxy_workspace, autolens_workspace, HowToGalaxy, HowToLens and both *_workspace_test repos found no other violating literal and no out-of-range axis_ratio conversion, so this is the complete literal surface.

Not in scope

The other red legs in that run are two different causes, neither fixed here:

  • The guides legs fail on Nautilus prior draws (e.g. np.float64(-0.7446446619131553)), not literals — already fixed by PyAutoGalaxy#568, which makes ModelParameterException a FitException so searches resample. That run started 18 hours before #568 merged.
  • The *_workspace_test aggregator legs fail on (1.0, 1.0) from PyAutoFit's mock scaffolding — filed as a PyAutoMind bug prompt.

🤖 Generated with Claude Code


Generated by Claude Code

The manual model-fitting exercise at the end of tutorial 3 shipped
`ell_comps=(0.5, 0.9)` as its deliberately-poor starting guess. That pair has
magnitude f = 1.0296, and the axis ratio is q = (1 - f) / (1 + f), so it implies
q = -0.015 — an ellipse with negative axis ratio, which has no geometric meaning.
The value was always unphysical; it only became visible when PyAutoGalaxy added
the `EllProfile` guard that rejects f >= 1.

Use `(0.5, 0.5)` instead: f = 0.707, q = 0.17. Still a wildly wrong starting
point against the simulator's true bulge (axis_ratio=0.9, angle=45deg, i.e.
ell_comps ~ (0.053, 0.0)), so the exercise keeps its "adjust these to improve
the fit" pedagogy, but the profile is now geometrically valid.

Applied to the script and to both generated mirrors (notebook, markdown) so the
scripts and notebooks smoke legs agree.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K4jSvqKzCMvqcF4b3wWXFq
@Jammy2211
Jammy2211 merged commit 07d8b04 into main Aug 13, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants