Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves numerical robustness and consistency in angle-related conversions and grid geometry operations, particularly targeting edge cases that can behave poorly in alternative numerical backends (e.g., JAX).
Changes:
- Added zero-component guards to multiple
arctan2-based conversions inautogalaxy/convert.pyusingxp.where. - Refactored
_cartesian_grid_via_radial_fromto compute trig terms viaangle_to_profile_grid_fromfor consistency.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| autogalaxy/profiles/geometry_profiles.py | Routes trig computation through angle_to_profile_grid_from when reconstructing Cartesian vectors from radial form. |
| autogalaxy/convert.py | Adds explicit handling for (0,0) component edge cases in ellipticity, shear, and multipole angle conversions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| angle = 0.5 * xp.arctan2(gamma_2, xp.where((gamma_1 == 0) & (gamma_2 == 0), 1.0, gamma_1)) * 180.0 / xp.pi | ||
| magnitude = xp.sqrt(gamma_1**2 + gamma_2**2) |
There was a problem hiding this comment.
Add regression coverage for the new (0,0) shear-components branch (e.g. shear_magnitude_and_angle_from(gamma_1=0.0, gamma_2=0.0, xp=jax.numpy)), asserting the returned magnitude/angle are finite and as expected. This ensures the backend-specific edge case this guard is addressing can’t regress.
| phi_m = xp.arctan2( | ||
| multipole_comps[0], | ||
| xp.where( | ||
| (multipole_comps[0] == 0) & (multipole_comps[1] == 0), | ||
| 1.0, | ||
| multipole_comps[1], | ||
| ), | ||
| ) * 180.0 / xp.pi / float(m) |
There was a problem hiding this comment.
Add regression coverage for the new (0,0) multipole-components handling (e.g. multipole_k_m_and_phi_m_from(multipole_comps=(0.0, 0.0), m=2, xp=jax.numpy)), checking the returned k_m / phi_m are finite and stable. This validates the intent of the xp.where guard across supported backends.
| angle = 0.5 * xp.arctan2(ell_comps[0], xp.where((ell_comps[0] == 0) & (ell_comps[1] == 0), 1.0, ell_comps[1])) | ||
| angle *= 180.0 / xp.pi |
There was a problem hiding this comment.
Add regression coverage for the new (0,0) elliptical-components branch. In particular, test that axis_ratio_and_angle_from(ell_comps=(0.0, 0.0)) returns finite values (axis_ratio ~ 1.0, angle ~ 0.0) for both NumPy and JAX backends (e.g. xp=jax.numpy) to ensure the original NaN/undefined-backend behavior can’t regress.
This pull request introduces improvements to the handling of elliptical component conversions and grid geometry in the codebase. The changes focus on making the angle calculation more robust and updating how grid angles are processed.
Elliptical component conversion improvements:
axis_ratio_and_angle_fromfunction inautogalaxy/convert.pyto handle cases where both elliptical components are zero, preventing division by zero in angle calculations. The calculation now usesxp.whereto substitute a value of1.0for the denominator when both components are zero.axis_ratio_and_angle_fromfunction definition for improved code readability.Grid geometry processing:
_cartesian_grid_via_radial_frommethod inautogalaxy/profiles/geometry_profiles.pyto use theangle_to_profile_grid_frommethod for calculatingcos_thetaandsin_theta, ensuring consistency and potentially improved handling of grid angles.