CHG: Assign event IDs to HDF5 data taken through the RTB

[fhagemann]

Addressing issue #215:

Data taken through the RTB has no native event IDs, but we might want to have event IDs for analysis purposes.
In this PR, I am adding consecutive event IDs (starting at 1 - as for all the data taken in unit-level data) to events loaded from an HDF5 file.

The way we can determine if data from a HDF5 file was taken through the RTB, we can check the JSON config, that has an entry "board":"rtb" or "board":"dib" as first entry. I've only seen the "board" field in the JSON config for HDFv2 data, and I've seen both upper-case and lower-case values (i.e both "DIB" and "dib") --> this is what informed the creation of the RegexPattern to determine m_DataTakenThroughRTB.

[fhagemann]

One thing that this PR might not tackle yet is combining data from multiple files, without restarting the event ID at zero (or maybe it does, I did not check this yet).

If the event ID resets to 1 with each file that's being read, this PR might require an additional commit.

I'll keep you posted :)

[fhagemann]

I also checked: in it's current implementation, nothing needs to be adjusted 😇 :
it does NOT reset the eventID counter to 0 when loading continuations of HDF5 files, but keeps on increasing the existing counter. So, this PR should also be good to go!

Looking forward to some reviews and getting this merged quickly! :)

[zoglauer]

This review was written by Claude (Anthropic's AI assistant) on behalf of @zoglauer.

Thanks for this! The approach makes sense, and the non-RTB path behaves exactly as before. I found one real problem and a few smaller things.

Main issue: the RTB event ID depends on a counter that only counts successful events

Event->SetID(m_NEventsInFile + 1) uses m_NEventsInFile, but m_NEventsInFile++ only runs at the very end of AnalyzeEvent(), after this early return:

if (Event->GetNStripHits() == 0) { ... return false; }

When an event is dropped there, the counter doesn't advance, and the next raw event gets the dropped event's ID. Examples are an event with only nearest-neighbor hits when IncludeNearestNeighbor == false, or one with no strip hit that maps to a detector. So the IDs are consecutive among the events that pass, not among the events in the file:

• Not reproducible: the same file gives different event IDs depending on loader options such as the nearest-neighbor setting. That breaks matching events across processing runs, which is the motivation in Adjust HDF5 parser for data taken through the RTB #215.
• Wrong meaning: m_NEventsInFile is a statistics counter printed in Finalize(), and the event ID shouldn't depend on it.

Suggestion: add a dedicated counter, e.g. m_RTBEventCounter. Reset it in Initialize(), increment it wherever a raw event is consumed (next to ++m_CurrentHit, before any early return), and call Event->SetID(m_RTBEventCounter). It still keeps counting across continuation files.

Smaller points

1. Unreachable error branch: the regex only matches RTB|rtb|DIB|dib, so the else { ... "Unknown board type" ... return false; } branch can never run. A simpler version: match "board"\s*:\s*"(\w+)" with std::regex::icase, lowercase the value, and keep the error for genuinely unknown boards. That also handles values like "Rtb". Right now those are silently treated as "no info → not RTB".
2. The flag is re-detected for every continuation file: OpenHDF5File() sets m_DataTakenThroughRTB again for each file. If a continuation file lacks the board field (the existing code notes that some GSE versions only wrote metadata to the first file), the flag flips to false mid-run. The rollover logic then restarts from a stale m_LastEventID, and IDs jump or collide. It's safer to only overwrite the flag when the field is actually present.
3. Style: the inner if block in the board detection is indented 4 spaces instead of 2. There's also trailing whitespace after bool m_DataTakenThroughRTB; in the header.
4. Tests: UTNModuleLoaderMeasurementsHDF exists but isn't extended. The test data in 406-1/*.hdf5 contains "board":"DIB", and the 542-1 files have no board field, so the DIB and "no info" paths can be tested now. Testing the RTB path needs a small RTB file in the test data.

The counter issue should be fixed before merging. Everything else is minor.

[ckierans]

Looking over Claude's review, I agree the counter issue should be fixed. The rest looks okay. Let me know when you've made the suggested changes and I can review again.

[fhagemann]

Working on it now!

[fhagemann]

It doesn't have unit tests yet, but I hope I addressed all the other comments.
Data with "board":true is still correctly identified as "taken through the RTB" with the updated regex BoardPattern.

[fhagemann]

This is probably not the most elegant way, but I created a "RTB"-like HDF5 file from the unit-test HDF5 file for 406-1, removed all event IDs and changed "board" to "RTB":

In the new unit tests RunRTBConfiguration, I am comparing the output from this modified file to the existing reference files that should add the event IDs back in.

@zoglauer Thoughts?

[ckierans]

I'm happy to merge, but maybe @zoglauer wants a check on how you've handed the unit tests for now?