Skip to content
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.

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

Muon parameters update #2670

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from
8 changes: 6 additions & 2 deletions src/ctapipe/containers.py
Original file line number Diff line number Diff line change
Expand Up @@ -1193,9 +1193,13 @@ class MuonParametersContainer(Container):
nan, "Sum of the pixel charges inside the integration area around a ring"
)
intensity_outside_ring = Field(nan, "Sum of the pixel charges outside the ring")
n_pixels_in_ring = Field(nan, "Number of pixels in the ring")
n_pixels_in_ring = Field(
nan,
"Number of pixels inside the ring integration area that passed the cleaning",
)
mean_intensity_outside_ring = Field(
nan, "Mean intensity of pixels outside the ring"
nan,
"Mean intensity of pixels inside the region limited by ring integration width and outer ring width.",
)
standard_dev = Field(
nan * u.deg,
Expand Down
6 changes: 4 additions & 2 deletions src/ctapipe/image/muon/features.py
Original file line number Diff line number Diff line change
Expand Up @@ -200,11 +200,13 @@ def ring_size_parameters(
ring_intensity: float
Sum of the p.e. inside the integration area of the ring
intensity_outside_ring: float
Sum of the photons outside the ring integration area that passed the cleaning
Sum of the p.e. outside the ring integration area that passed the cleaning
n_pixels_in_ring: int
Number of pixels inside the ring integration area that passed the cleaning
mean_intensity_outside_ring: float
Mean intensity of the pixels outside the ring, but still close to it
Mean intensity of the pixels outside the integration area of the ring,
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this the same as l1201 of ctapipe.containers? If so, why the docstrings are different?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They are the same, but I didn't want to duplicate the exact same text.
Or should I just duplicate the doc strings from ctapipe.containers into the doc strings in the methods from ctapipe.image.muon.features?

Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps this function simply should return MuonParametersContainer?
Also, the number of input parameters is very high, consider to wrap them into a structure (dict, container, ...)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps this function simply should return MuonParametersContainer?

All this module in fact is returning just one MuonParametersContainer and I adopted already existing style where each method returning some logically/technically connected part of MuonParametersContainer. Should I reconsider this and return calculated parts of MuonParametersContainer from every method?

Also, the number of input parameters is very high, consider to wrap them into a structure (dict, container, ...)

Good point

and restricted by the outer ring width, i.e. in the strip between
ring integration width and outer ring width.
"""

dist = np.sqrt((pixel_x - center_x) ** 2 + (pixel_y - center_y) ** 2)
Expand Down
Loading