-
Notifications
You must be signed in to change notification settings - Fork 0
Geomag ctao sorting #39
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
Conversation
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.
Pull request overview
This PR adds CTAO-specific support for telescope indexing/sorting and geomagnetic angle calculation by introducing an observatory configuration, new geomagnetic field presets, and updated sorting behavior (mirror area first, then size).
Changes:
- Add
observatoryplumbing through config/model application into feature flattening and geomagnetic angle computation. - Introduce CTAO-NORTH/CTAO-SOUTH geomagnetic field components and rename the geomag API parameter to
observatory. - Adjust telescope sorting logic (area desc, then size desc) and add tests around CTAO/ImgSel_list behavior.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_sort_logic.py | Adds unit tests for the intended sort ordering (currently re-implements the algorithm inline). |
| tests/test_imgsel_debug.py | Updates the CTAO ImgSel_list-mode test to pass observatory. |
| src/eventdisplay_ml/models.py | Propagates observatory into flatten_feature_data for apply-time inference. |
| src/eventdisplay_ml/hyper_parameters.py | Changes default regression pre-cut to require DispNImages >= 2. |
| src/eventdisplay_ml/geomag.py | Adds CTAO field presets and changes geomag angle API to use observatory. |
| src/eventdisplay_ml/features.py | Adds ImgSel_list to feature/branch lists for regression/classification. |
| src/eventdisplay_ml/data_processing.py | Adds observatory parameter, switches indexing-mode selection logic, and changes sorting implementation. |
| src/eventdisplay_ml/config.py | Adds CLI --observatory to training/apply configuration. |
Comments suppressed due to low confidence (1)
src/eventdisplay_ml/data_processing.py:404
- The indexing-mode selection is now driven solely by the
observatorystring. This is brittle: ifobservatoryis mis-set (or defaults) for a dataset, the code will try to readImgSel_listand fail (or pick the wrong normalization). The previous data-driven detection via presence ofR_core/ImgSel_listis safer; consider using column/field presence as the primary signal andobservatoryonly as an override.
# Index remapping mode for CTAO-style variable-length indexing
index_list_for_remapping = None # None = telescope-ID-indexed (VERITAS R_core mode)
if observatory.lower() == "veritas":
_logger.info("Detected VERITAS R_core fixed telescope-ID indexing mode.")
else:
_logger.info("Detected CTAO ImgSel_list variable-length indexing mode.")
index_list_for_remapping = _to_dense_array(df["ImgSel_list"])
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
No description provided.