Skip to content
Draft
Changes from 2 commits
Commits
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
91 changes: 91 additions & 0 deletions src/ctapipe/image/muon/intensity_fitter.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,97 @@
SQRT2 = np.sqrt(2)


def polygon_chord(mu_x, mu_y, phi, ri_x, ri_y, vi_x, vi_y):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This function should be njit, as it will be called. If possible, it would also be good if it could be vectorized over multiple tiles.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, I see. Let me finish implementing it with multiple tiles.

"""
Compute the chord length of a ray intersecting a polygon.

This function calculates the length of the segment formed by the intersection
of a ray (defined by an origin and direction) with a polygon defined by its
edges. The polygon is represented parametrically using starting points and
direction vectors for each edge.

Parameters
----------
mu_x : float

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These should be quantities, as we discussed offline, the best would be to use telescope coordinate system lat/lon.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Also, perhaps tuples of (x,y) or arrays of them would be more straightforward to define points (arrays of points)

X-coordinate of the ray origin, which is the muon's impact point (x).
mu_y : float
Y-coordinate of the ray origin, which is the muon's impact point (y).
phi : float
Angle defining the direction of the ray.
ri_x : ndarray of shape (N,)
X-coordinates of the starting points of the polygon edges.
ri_y : ndarray of shape (N,)
Y-coordinates of the starting points of the polygon edges.
vi_x : ndarray of shape (N,)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we should deduce these (if needed) from the vertices. You can write a helper for this.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes, I'd also prefer the vertices as input. But maybe the function as it is here is fine and that transformation is made once outside of this function.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes agree with @maxnoe . This is a low-level function. Let's keep it as it is, since all the necessary computations should be handled by the higher-level function that calls it.

X-components of the edge direction vectors.
vi_y : ndarray of shape (N,)
Y-components of the edge direction vectors.

Returns
-------
float

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should be also a unit of length (or angular, equivalent in the telescope frame)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Every branch returns (length, x_int, y_int) and not only a float. This should be reflected in the docstring.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It is indeed only a float. The function is not vectorized.

Length of the chord formed by the intersection of the ray with the polygon:
- 0.0 if there is no intersection,
- distance from the ray origin to the intersection point if only one intersection,
- distance between two intersection points if exactly two intersections,
- accumulated segment length for multiple intersections (handles complex polygons).

Notes
-----
- The ray is defined parametrically as:
(x, y) = (mu_x, mu_y) + s * (cos(phi), sin(phi)), with s >= 0
- Each polygon edge is defined as:
(x, y) = (ri_x, ri_y) + t * (vi_x, vi_y), with 0 <= t < 1
- The function computes intersections by solving a 2D linear system.
- A small epsilon (`1e-20`) is added to the denominator to avoid division by zero.
- For multiple intersections, distances are sorted and combined with alternating
signs to compute the total chord length (useful for non-convex polygons).

Examples
--------
>>> length = polygon_chord(mu_x=0, mu_y=0, phi=np.pi/4,
... ri_x=..., ri_y=..., vi_x=..., vi_y=...)
>>> print(length)
"""

# Effective speed of the ray, with unit norm.
vmu_x = np.cos(phi)
vmu_y = np.sin(phi)

epsilon_d = 1.0e-20

c1 = mu_x - ri_x
c2 = mu_y - ri_y
D = vi_x * vmu_y - vi_y * vmu_x + epsilon_d

t = ( c1 * vmu_y - c2 * vmu_x ) / D
s = ( vi_y * c1 - vi_x * c2 ) / D

status = np.column_stack((vi_x, ri_x, vi_y, ri_y, t, s))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why do you column_stack this (making a copy) just to then index into the individual comments making it basically unreadable what you are accessing?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, thank you. It can be removed. Initially, I did it to have everything in the same 2D array, but you’re right, it can be removed.

mask = (status[:,4] >= 0) & (status[:,4] < 1) & (status[:,5] >= 0)

x_int = status[mask][:,0]*status[mask][:,4] + status[mask][:,1]
y_int = status[mask][:,2]*status[mask][:,4] + status[mask][:,3]

if x_int.shape[0] == 0 :

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I suggest to return the same type (an array) for every if loop branch. I don't say that the code is wrong, but returning the same type will make it cleaner.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It is indeed only a float. The function is not vectorized. All of them return only one number.

return 0.0
elif x_int.shape[0] == 1:
return np.squeeze(np.sqrt((x_int - mu_x) ** 2 + (y_int - mu_y) ** 2))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

please do not duplicate computation of the same things in different branches.

compute first, then return, i.e.

chord_length = ...
if return intersections:
    return chord_length, ...
return chord_length

elif x_int.shape[0] == 2:
return np.squeeze(np.sqrt((x_int[0] - x_int[1]) ** 2 + (y_int[0] - y_int[1]) ** 2))
else:
dist = np.sort(
np.squeeze(np.sqrt((x_int - mu_x) ** 2 + (y_int - mu_y) ** 2))
)
sign_arr = np.ones(x_int.shape[0])
if x_int.shape[0] % 2 == 0:
sign_arr[0::2] = -1
return np.sum(dist * sign_arr)

sign_arr[1::2] = -1
return np.sum(dist * sign_arr)


def chord_length(radius, rho, phi, phi0=0):
"""
Function for integrating the length of a chord across a circle (effective chord length).
Expand Down
Loading