-
Notifications
You must be signed in to change notification settings - Fork 25
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
Effective Area 3D #281
base: main
Are you sure you want to change the base?
Effective Area 3D #281
Conversation
…ensions The different methods use binning in different coordinate systems - effective_area_3D_polar uses a polar coordinate system with radial FOV offset and azimuthal FOV position angle bins - effective_area_3D_nominal uses a nominal coordinate system with FOV longitude and FOV latitude bins
These calculate the number of showers in each bin for different coordinate systems - calculate_n_showers_3D_polar uses a polar coordinate system with radial FOV offset bins and azimuthal FOV position angle bins - calculate_n_showers_3D_nominal uses a nominal coordinate system with FOV longitude and latitude bins
…into effective_area_3d
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #281 +/- ##
==========================================
- Coverage 95.60% 95.01% -0.60%
==========================================
Files 62 62
Lines 3278 3308 +30
==========================================
+ Hits 3134 3143 +9
- Misses 144 165 +21 ☔ View full report in Codecov by Sentry. |
in the sky. | ||
""" | ||
phi = position_angle( | ||
events["pointing_az"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure this definition matches the defintion in GADF given here:
https://gamma-astro-data-formats.readthedocs.io/en/v0.3/general/coordinates.html#field-of-view
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add tests for that
fov_offset_bins: astropy.units.Quantity[angle] | ||
The field of view radial bin edges in which to calculate effective area. | ||
fov_position_angle_bins: astropy.units.Quantity[radian] | ||
The field of view azimuthal bin edges in which to calculate effective area. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure if this could lead to confusion, but a 2D azimuthal coordinate and position angle are not quite the same: the polar azimuth starts on the positive X-axis and goes counter-clockwise, while position angle starts on the positive Y-axis and goes clockwise, so there is a 90 deg phase shift and different direction. Perhaps just to avoid this, say "Field-of-view azimuthal (from Y-axis, toward positive x) bin edges...". Probably this should be shown as a figure somewhere in the general PyIRF documentation...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is going into the same direction as the comment above about being clear how we define these coordinate systems and adding tests for the cases spelled out by GADF.
One confusing thing here is that the position angle is defined relative to the original coordinate system axes (AltAz or RaDec) but the FoV lon has the opposite direction to the original lon. Which I think makes the polar angle go counter-clockwise when looking at in a common plot of field of view coordinates where the cartesian like axes are FoV lon/lat...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understand it correctly the GADF FOV position angle definition is exactly the same as the usual definition of the azimuthal angle in polar coordinates, right? (Angle increasing counter-clockwise)
So following this definition there shouldn't technically be a difference between the two, although I understand that just calling it the azimuthal angle here might still cause confusion.
Since we are following the GADF conventions anyways, would it be agreeable to just change "field of view azimuthal bin edges" to "field of view position angle bin edges" and maybe showing a figure somewhere to clarify as suggested?
Adds two methods of calculating the effective area in bins of energy and two spacial dimensions. These methods differ in the coordinate frame used for the spacial dimensions.
calculate_effective_area_3d_polar
uses a polar coordinate system with FOV pointing position offset and position angle coordinatescalculate_effective_area_3d_nominal
uses a quasi-cartesian coordinate system with FOV longitude and latitude coordinates centred on the pointing positionAlso adds respective functions to calculate the number of showers expected in the 3D bins from simulation info to the SimulatedEventsInfo class (
calculate_n_showers_3d_polar
,calculate_n_showers_3d_nominal
), as well as a utility function to calculate the position angle w.r.t. the pointing from an event table (calculate_source_fov_position_angle
).