Skip to content

feature/geometry_hot_fix #274

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Jammy2211 merged 1 commit into from
Feb 7, 2026
Merged

feature/geometry_hot_fix #274

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions autogalaxy/profiles/geometry_profiles.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,9 @@ def _cartesian_grid_via_radial_from(
The circular radius of each coordinate from the profile center.
"""
grid_angles = xp.arctan2(grid.array[:, 0], grid.array[:, 1])
cos_theta, sin_theta = self.angle_to_profile_grid_from(
grid_angles=grid_angles, xp=xp
)

cos_theta = xp.cos(grid_angles)
sin_theta = xp.sin(grid_angles)
Comment on lines +109 to +110
Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_cartesian_grid_via_radial_from previously delegated to self.angle_to_profile_grid_from(...), which is overridden by EllProfile to account for the profile’s position angle. Replacing it with xp.cos/sin(grid_angles) bypasses that override and changes behavior for any subclass with a non-zero angle that calls this helper. Consider restoring the call to self.angle_to_profile_grid_from(...), or (if the intent is to always ignore the profile angle because the grid is already in the profile reference frame) add an explicit comment and/or use a clearly-named helper to avoid silently breaking polymorphism.

Suggested change
cos_theta = xp.cos(grid_angles)
sin_theta = xp.sin(grid_angles)
# Use angle_to_profile_grid_from so subclasses (e.g. with non-zero position angle)
# can override how angles are converted to Cartesian directions.
cos_theta, sin_theta = self.angle_to_profile_grid_from(
grid_angles=grid_angles, xp=xp, **kwargs
)

Copilot uses AI. Check for mistakes.

return xp.multiply(radius[:, None], xp.vstack((sin_theta, cos_theta)).T)
Comment on lines 107 to 112
Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change alters the angle-to-(sin,cos) conversion path used by _cartesian_grid_via_radial_from, but there’s no unit test covering this helper (e.g. verifying the returned Cartesian vectors for known input grids/radii, including a rotated EllProfile case if overrides are expected to apply). Adding a focused test would help prevent regressions in coordinate conversion.

Copilot uses AI. Check for mistakes.

Expand Down
2 changes: 1 addition & 1 deletion autogalaxy/profiles/mass/total/isothermal.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ def deflections_yx_2d_from(self, grid: aa.type.Grid2DLike, xp=np, **kwargs):
"""
return self._cartesian_grid_via_radial_from(
grid=grid,
radius=xp.full(grid.shape[0], 2.0 * self.einstein_radius_rescaled(xp)),
xp=xp,
radius=xp.full(grid.shape[0], 2.0 * self.einstein_radius_rescaled(xp)),
**kwargs,
)
Loading