Incident angle model parameters - #2543
EshitaJoshi wants to merge 16 commits into
Conversation
…mtools into add-new-model-params
There was a problem hiding this comment.
🟡 Changes recommended
The current export path can write empty model-parameter tables for non-finite data, and the documentation has an RST directive block that needs to be fenced properly to avoid Sphinx rendering/build issues.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 9/9 changed files
- Comments generated: 3
- Review effort level: Lite
GernotMaier
left a comment
GernotMaier
left a comment
There was a problem hiding this comment.
See below, probably me not understanding what is going on.
| "calculate_primary_secondary_angles", | ||
| dest="calculate_primary_secondary_angles", | ||
| help="Compute angles of incidence on primary and secondary mirrors", | ||
| help="Compute angles of incidence on primary and secondary mirrors (enabled by default)", |
There was a problem hiding this comment.
Is this true by default, even for single-mirror telescopes? I am trying to understand what the impact is. Does it e.g., impact the reading of columns using _default_column_indices?
| out=np.zeros_like(detected_counts, dtype=float), | ||
| where=bin_areas > 0, | ||
| ) | ||
| max_efficiency = np.max(efficiency) |
There was a problem hiding this comment.
I do struggle quite a bit with this part. Is this really light guide efficiency? Where does "photons transmitted for the light guide / photons incident on light-guide entrance" is applied? Is it possibly that this is only the angle distribution of photons incident on the light-guide entrance?
|
|
@GernotMaier thanks for the review - can you take another look at this please? |
Update and cleanup
simtools-derive-incident-angles, and add derivation and export oflightguide_efficiency_vs_incidence_angle.Closes #1976