Update LAPPD ID handling to support LAPPD INCOM IDs#381
Update LAPPD ID handling to support LAPPD INCOM IDs#381marc1uk merged 4 commits intoANNIEsoft:Applicationfrom
Conversation
Motivation: Newer runs use Manufacturer (Incom) IDs instead of ACC IDs, which breaks compatibility with existing downstream tools expecting ACC IDs. Changes: - Added a structure in DataModel/PsecData.h to store LAPPD ID mapping information loaded from configfiles/LAPPDProcessedAna/LAPPDIDConfig.csv in UserTools/LAPPDLoadStore/LAPPDLoadStore.cpp. - Implemented lookup logic to retrieve the appropriate mapping for a given run number and ACC ID, returning the corresponding Manufacturer ID and position. - Added reverse mapping function to convert Manufacturer IDs back to ACC IDs for compatibility with downstream tools. - Removed redundant structure previously defined in ANNIEEventTreeMaker.h including some fixes. Heuristic: - If LAPPD_ID > 20, it is treated as a Manufacturer (Incom) ID (based on current detector configuration assumptions). Notes / Limitations: - Current implementation relies on a heuristic (ID > 20) to distinguish ID types. - This assumption is based on current detector configuration and observed ID ranges. - Planned improvement: replace heuristic with run-dependent or database-driven mapping. - Mapping file (LAPPDIDConfig.csv) must be updated as detector configuration evolves. Validation: - Basic functionality verified (toolchain runs end-to-end). - Detailed validation of Event Building + data processing before-and-after is pending.
- Update ID mapping logic for consistent handling across the toolchain. - Store Pulse IDs and Hit IDs as INCOM IDs.
|
This is great to see you've added the LAPPD ID and position mapping, this will be super helpful! I also appreciate the detailed description on the individual commits, the validation testing, and the note on limitations / future work. For future PRs and ease of readability I would recommend just adding those comments to the main PR and also filling out the checklist. No obvious problems stand out to me. You note that the "detailed validation of Event Building + data processing before-and-after is pending" - once that is verified I think its good to go and I will give my stamp of approval. |
S81D
added
Ready for Level 0 Review and merge
waiting for submitter
and removed
Ready for Level 0 Review and merge
labels
|
Thanks, Steven, for reviewing this. I don’t want to take credit for the original work, most of it was done by Yue last year but wasn’t merged into the latest ToolAnalysis release. I’ve applied a minimal version of his changes along with few modifications to get a working EventBuildingV2 and associated toolchains. For validation of all data processing steps, I’ve tested three part-files, and everything works as expected. The LAPPD data before and after these changes remains unchanged. Let me know if this is enough for the approval. |
|
Great! I will mark it as ready for level 0 review and merging. |
|
Looks good, thanks Anuj. |
3 participants
Describe your changes
Checklist before submitting your PR
newusage, there is a reason the data must be on the heapnewthere is adelete, unless I explicitly know why (e.g. ROOT or a BoostStore takes ownership)Additional Material
Attach any validation or demonstration files here. You may also link to relavant docdb articles.