Overview
The two mirror grid-transform methods in GeometryProfile use inconsistent spherical-profile name checks: transformed_to_reference_frame_grid_from checks endswith("Sph") while transformed_from_reference_frame_grid_from checks startswith("Sph"). Spherical profiles are suffix-named (IsothermalSph, NFWSph, …) and no concrete class starts with "Sph", so the inverse transform's spherical branch never fires — every Sph profile takes the elliptical path with a pointless identity rotation. Numerically benign today (Sph profiles have angle = 0), but it is an asymmetric latent trap and wasted work on the hot path.
Plan
- Fix the name-check asymmetry in
transformed_from_reference_frame_grid_from so both transforms take the translation-only path for spherical profiles.
- Evaluate replacing both string checks with an
isinstance-based check so subclasses like IsothermalSphMLR (name ends "MLR", missed by both string checks) are also covered.
- Add a regression unit test round-tripping a grid through both transforms for a spherical profile (NumPy only, no JAX).
- Run the profile/geometry test suite and ship via the standard library flow.
Detailed implementation plan
Affected Repositories
Branch Survey
| Repository |
Current Branch |
Dirty? |
| ./PyAutoGalaxy |
main |
clean |
Suggested branch: feature/sph-transform-name-check
Implementation Steps
autogalaxy/profiles/geometry_profiles.py:392 — transformed_from_reference_frame_grid_from checks self.__class__.__name__.startswith("Sph"); its mirror transformed_to_reference_frame_grid_from (line 371) checks endswith("Sph"). Minimal fix: change line 392 to endswith("Sph") for symmetry.
- Preferred fix (evaluate first): replace both name checks with an
isinstance(self, SphProfile)-style check if the class hierarchy supports it cleanly. This also covers IsothermalSphMLR (autogalaxy/profiles/scaling_relations.py:16, subclasses mp.IsothermalSph but its name ends "MLR" so both string checks miss it). Precedent for pairing the name check with isinstance exists at autogalaxy/profiles/light/linear/abstract.py:131.
- Add a regression unit test: for a spherical profile, round-trip a grid through
transformed_to_reference_frame_grid_from → transformed_from_reference_frame_grid_from and assert the translation-only path is taken both ways / results are exact. NumPy only — no JAX in unit tests.
- Run the geometry/profiles tests, then
ship_library.
Key Files
autogalaxy/profiles/geometry_profiles.py — the asymmetric checks (lines 371 / 392).
autogalaxy/profiles/scaling_relations.py — IsothermalSphMLR, the subclass both string checks miss.
autogalaxy/profiles/light/linear/abstract.py — existing endswith("Sph") or isinstance(...) precedent.
Notes
- Numerical impact currently nil: spherical profiles have
angle = 0, so the elliptical path applies an identity rotation. This is a latent-trap/perf fix; expected zero downstream workspace impact.
Original Prompt
Click to expand starting prompt
The following line in @autogalaxy/profiles/geometry_profiles.py is probbaly a bug
Type: bug
Target: PyAutoGalaxy
Difficulty: small
Autonomy: safe
Priority: normal
Status: formalised
The following line in @autogalaxy/profiles/geometry_profiles.py is probbaly a bug:
Overview
The two mirror grid-transform methods in
GeometryProfileuse inconsistent spherical-profile name checks:transformed_to_reference_frame_grid_fromchecksendswith("Sph")whiletransformed_from_reference_frame_grid_fromchecksstartswith("Sph"). Spherical profiles are suffix-named (IsothermalSph,NFWSph, …) and no concrete class starts with "Sph", so the inverse transform's spherical branch never fires — every Sph profile takes the elliptical path with a pointless identity rotation. Numerically benign today (Sph profiles haveangle = 0), but it is an asymmetric latent trap and wasted work on the hot path.Plan
transformed_from_reference_frame_grid_fromso both transforms take the translation-only path for spherical profiles.isinstance-based check so subclasses likeIsothermalSphMLR(name ends "MLR", missed by both string checks) are also covered.Detailed implementation plan
Affected Repositories
Branch Survey
Suggested branch:
feature/sph-transform-name-checkImplementation Steps
autogalaxy/profiles/geometry_profiles.py:392—transformed_from_reference_frame_grid_fromchecksself.__class__.__name__.startswith("Sph"); its mirrortransformed_to_reference_frame_grid_from(line 371) checksendswith("Sph"). Minimal fix: change line 392 toendswith("Sph")for symmetry.isinstance(self, SphProfile)-style check if the class hierarchy supports it cleanly. This also coversIsothermalSphMLR(autogalaxy/profiles/scaling_relations.py:16, subclassesmp.IsothermalSphbut its name ends "MLR" so both string checks miss it). Precedent for pairing the name check withisinstanceexists atautogalaxy/profiles/light/linear/abstract.py:131.transformed_to_reference_frame_grid_from→transformed_from_reference_frame_grid_fromand assert the translation-only path is taken both ways / results are exact. NumPy only — no JAX in unit tests.ship_library.Key Files
autogalaxy/profiles/geometry_profiles.py— the asymmetric checks (lines 371 / 392).autogalaxy/profiles/scaling_relations.py—IsothermalSphMLR, the subclass both string checks miss.autogalaxy/profiles/light/linear/abstract.py— existingendswith("Sph") or isinstance(...)precedent.Notes
angle = 0, so the elliptical path applies an identity rotation. This is a latent-trap/perf fix; expected zero downstream workspace impact.Original Prompt
Click to expand starting prompt
The following line in @autogalaxy/profiles/geometry_profiles.py is probbaly a bug
Type: bug
Target: PyAutoGalaxy
Difficulty: small
Autonomy: safe
Priority: normal
Status: formalised
The following line in @autogalaxy/profiles/geometry_profiles.py is probbaly a bug:
if self.class.name.startswith("Sph"):
Can you check you agree and apply a fix.