Overview
mge_model_from constructs centre and ellipticity priors once outside the per-basis loop, so every basis shares the same prior instances. This means gaussian_per_basis > 1 adds redundant Gaussians with zero extra flexibility — the model always has 4 free params (2 centre + 2 ell_comps) regardless of how many bases are requested. This is a refactor regression; the parameter was meant to add model flexibility (e.g. twisting isophotes).
The fix moves ell_comps construction inside the loop (unconditionally) and adds a centre_per_basis flag to optionally give each basis independent centres.
Plan
- Verify the bug empirically (confirmed:
gaussian_per_basis=2 produces 4 free params, same as =1)
- Move ell_comps prior construction inside the per-basis loop so each basis gets independent ellipticity
- Add
centre_per_basis: bool = False parameter — when True, each basis gets independent centre priors
- Update the docstring to document per-basis coupling semantics
- Add unit tests for all parameter count scenarios
- Grep workspace callers to confirm none rely on the buggy tied-ell_comps behaviour
Detailed implementation plan
Affected Repositories
Work Classification
Library
Branch Survey
| Repository |
Current Branch |
Dirty? |
| PyAutoGalaxy |
main |
2 untracked files |
Suggested branch: feature/mge-fix
Worktree root: ~/Code/PyAutoLabs-wt/mge-fix/
Implementation Steps
- Add
centre_per_basis: bool = False to the mge_model_from signature (after centre_fixed)
- Keep shared centre construction outside the loop (default
centre_per_basis=False)
- Move
ell_comps_0/1 construction inside the for j in range(gaussian_per_basis) loop — each iteration creates fresh prior objects
- When
centre_per_basis=True, also construct centre priors inside the loop
- Leave
centre_fixed handling and PYAUTO_WORKSPACE_SMALL_DATASETS guard unchanged
- Update the docstring: fix stale param names, document
centre_per_basis, explain per-basis coupling
- Add unit tests covering:
gaussian_per_basis=1 (elliptical=4, spherical=2), =2 default (6), =2 centre_per_basis=True (8), =3 (8), =2 spherical (2), =2 spherical centre_per_basis=True (4), centre_fixed with =2 (4 ell_comps only)
- Run
pytest test_autogalaxy/analysis/test_model_util.py -x
Key Files
autogalaxy/analysis/model_util.py — fix prior construction in mge_model_fromtest_autogalaxy/analysis/test_model_util.py — new unit tests
Follow-up (separate task)
SLaM scripts at autolens_workspace/scripts/multi/features/slam/{independent,simultaneous}.py use total_gaussians=20, gaussian_per_basis=2 and may want total_gaussians=30.
Original Prompt
Click to expand starting prompt
Investigate and fix bug in mge_model_from where gaussian_per_basis > 1 does not produce independent ell_comps per basis. Centre and ellipticity priors are constructed once outside the loop, tying all bases. Fix by moving ell_comps inside the loop and adding centre_per_basis flag. ~60+ workspace callers with gaussian_per_basis=2 will now get the intended extra flexibility.
Overview
mge_model_fromconstructs centre and ellipticity priors once outside the per-basis loop, so every basis shares the same prior instances. This meansgaussian_per_basis > 1adds redundant Gaussians with zero extra flexibility — the model always has 4 free params (2 centre + 2 ell_comps) regardless of how many bases are requested. This is a refactor regression; the parameter was meant to add model flexibility (e.g. twisting isophotes).The fix moves ell_comps construction inside the loop (unconditionally) and adds a
centre_per_basisflag to optionally give each basis independent centres.Plan
gaussian_per_basis=2produces 4 free params, same as=1)centre_per_basis: bool = Falseparameter — when True, each basis gets independent centre priorsDetailed implementation plan
Affected Repositories
Work Classification
Library
Branch Survey
Suggested branch:
feature/mge-fixWorktree root:
~/Code/PyAutoLabs-wt/mge-fix/Implementation Steps
centre_per_basis: bool = Falseto themge_model_fromsignature (aftercentre_fixed)centre_per_basis=False)ell_comps_0/1construction inside thefor j in range(gaussian_per_basis)loop — each iteration creates fresh prior objectscentre_per_basis=True, also construct centre priors inside the loopcentre_fixedhandling andPYAUTO_WORKSPACE_SMALL_DATASETSguard unchangedcentre_per_basis, explain per-basis couplinggaussian_per_basis=1(elliptical=4, spherical=2),=2default (6),=2 centre_per_basis=True(8),=3(8),=2 spherical(2),=2 spherical centre_per_basis=True(4),centre_fixedwith=2(4 ell_comps only)pytest test_autogalaxy/analysis/test_model_util.py -xKey Files
autogalaxy/analysis/model_util.py— fix prior construction inmge_model_fromtest_autogalaxy/analysis/test_model_util.py— new unit testsFollow-up (separate task)
SLaM scripts at
autolens_workspace/scripts/multi/features/slam/{independent,simultaneous}.pyusetotal_gaussians=20, gaussian_per_basis=2and may wanttotal_gaussians=30.Original Prompt
Click to expand starting prompt
Investigate and fix bug in
mge_model_fromwheregaussian_per_basis > 1does not produce independent ell_comps per basis. Centre and ellipticity priors are constructed once outside the loop, tying all bases. Fix by moving ell_comps inside the loop and addingcentre_per_basisflag. ~60+ workspace callers withgaussian_per_basis=2will now get the intended extra flexibility.