Refactor event building to use DetectorId instead of ChannelId#31
Refactor event building to use DetectorId instead of ChannelId#31
Conversation
Aligns event building with new DSP logic where data is keyed by detector ID. Changes: - Rename flatten_over_channels to flatten_over_detectors - Replace channel/chevtno columns with detector/detevtno - Store detector as DetectorId type instead of Int - Update build_global_events to use DetectorIdLike signatures - Refactor calibrate_all.jl to use detector-based access - Remove unnecessary only.() wrapper from max_e_det
|
There is still a lot of chdata naming in there but this is not necessary to change now. If we want to do this consistent we also have to change the prop fucntions names in LegendDataManagement evt_fucntions.jl and calibration_functions.jl. This is just for the compability with the new DSP tier format. I already tested this version, everything is working. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #31 +/- ##
=====================================
Coverage 5.77% 5.77%
=====================================
Files 7 7
Lines 329 329
=====================================
Hits 19 19
Misses 310 310 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
I would prefer the naming to be consistent. So if it is not too much of work, let's use |
theHenks
left a comment
theHenks
left a comment
There was a problem hiding this comment.
I suggest renaming the other funtions too, such as calibrate_geds, calibrate_spms, calibrate_pmts etc.
- Rename calibrate_*_channel_data functions to calibrate_*_detector_data - Rename chdata/channel_data variables to detdata/detector_data - Rename prop functions - (jldataprod and LegendDataManagement changed too)
There was a problem hiding this comment.
Pull request overview
This PR refactors the event building system to use DetectorId instead of ChannelId as the primary key for data organization, aligning with new DSP logic where data is keyed by detector ID rather than channel ID.
Changes:
- Renamed
flatten_over_channelstoflatten_over_detectorswith corresponding column name changes (channel/chevtno → detector/detevtno) - Updated all calibration functions to use detector-based naming and
DetectorIdtypes - Modified
calibrate_all.jlto access datastores using detector IDs and updated all variable and function references - Changed
build_global_eventsto useDetectorIdLikesignatures throughout
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/flatten_over_channels.jl | Renamed function and updated all references from channel to detector terminology; changed column names and types to use DetectorId |
| src/calibrate_smps.jl | Renamed calibrate_spm_channel_data to calibrate_spm_detector_data; updated parameter names and internal variable references |
| src/calibrate_pmts.jl | Renamed calibrate_pmt_channel_data to calibrate_pmt_detector_data; updated parameter names and internal variable references |
| src/calibrate_geds.jl | Renamed calibrate_ged_channel_data to calibrate_ged_detector_data; updated parameter names and internal variable references |
| src/calibrate_aux.jl | Renamed calibrate_aux_channel_data to calibrate_aux_detector_data; updated parameter names and internal variable references |
| src/calibrate_all.jl | Refactored to use detector-based access patterns throughout; changed datastore access from ChannelId to string(DetectorId); updated variable names and function calls |
| src/build_global_events.jl | Updated function signatures to use DetectorIdLike; renamed columns from channel/chevtno to detector/detevtno; updated documentation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
DaGeibl
left a comment
DaGeibl
left a comment
There was a problem hiding this comment.
I think the changes look good.
Only real change besides renaming is switching from storing channels as Int to DetectorID. If you tested that everything runs, that should be fine.
4 participants
Aligns event building with new DSP logic where data is keyed by detector ID.
Changes: