Skip to content

Address high-priority bugs flagged by Claude - #398

Merged
marc1uk merged 9 commits into
ANNIEsoft:Applicationfrom
anuj-guptta:claude-fixes-2
Oct 21, 2026
Merged

marc1uk merged 9 commits into
ANNIEsoft:Applicationfrom
anuj-guptta:claude-fixes-2

Conversation

@anuj-guptta

Copy link
Copy Markdown
Contributor

Describe your changes

This PR contains a subset of fixes addressing high-priority bugs flagged via Andy's Claude. These changes focus on safeguarding data consistency and ensuring reliable processing during the decoder stage.

Documented here: https://annie-docdb.fnal.gov/cgi-bin/sso/ShowDocument?docid=7024

  • EBLAPPD: Initialise thisRunNum to -1

  • EBLoadRaw

    • On a jump to the next file due to LAPPD data, pause all decoders and set NewRawDataEntryAccessed so nothing reprocesses old data or stops.
    • Reset LAPPDTotalEntries when loading LAPPD data from a new file. Also fixed in LoadRawData tool.
    • If no trigger overlap store exists, read the last real CTC entry instead of reusing the previous one.
  • EBTriggerGrouper

    • A group still open at the end of the buffer had its triggers erased without being saved, losing that group when it spanned two Execute() calls. Now its triggers stay in the buffer and the group is completed on the next pass.
    • Change the default CosmicTriggerMain from 45 to 36 (matches the cosmic config).
  • LoadGeometry: Strip trailing '\r' when reading geometry CSV files.

  • PhaseIIADCHitFinder

    • With a relative threshold, the baseline was added to the threshold in every minibuffer without resetting it, so it kept growing. Now each minibuffer starts from the database threshold.
    • When a channel was first added to the pulse map, the whole pulse_vec was inserted and later pieces were appended again (duplicates for channels with more than one waveform piece). Now only the current piece is inserted.
  • LAPPDTreeMaker: Initialise TriggerWordMap to nullptr.

  • LAPPDLoadStore

    • queryNearestACCID: log an error and stop the toolchain if no ACC ID is found for the LAPPD manufacturer ID.
    • Set NewLAPPDDataAvailable only when the event was actually saved, and reset it at the start of each Execute().
    • Reject events with an uneven number of boards.
    • Clear LAPPDLoadedTimeStamps in CleanDataObjects().

Checklist before submitting your PR

  • This PR implements a single change (one new/modified Tool, or a set of changes to implement one new/modified feature)
  • This PR alters the minimum number of files to affect this change
  • If this PR includes a new Tool, a README and minimal demonstration ToolChain is provided
  • If a new Tool/ToolChain requires model or configuration files, their paths are not hard-coded, and means of generating those files is described in the readme, with examples provided on /pnfs/annie/persistent
  • For every new usage, there is a reason the data must be on the heap
  • For every new there is a delete, 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.

…ger overlap

- On a jump to the next file due to LAPPD data, pause all decoders and
  set NewRawDataEntryAccessed so nothing reprocesses old data or stops.
- Reset LAPPDTotalEntries when loading LAPPD data from a new file.
  - Also fixed in LoadRawData tool.
- If no trigger overlap store exists, read the last real CTC entry
  instead of reusing the previous one.
- A group still open at the end of the buffer had its triggers erased
  without being saved, losing that group when it spanned two Execute()
  calls. Now its triggers stay in the buffer and the group is completed
  on the next pass.
- Change the default CosmicTriggerMain from 45 to 36 (matches the cosmic
  config)
…fix pulse map fill

- With a relative threshold, the baseline was added to the threshold in
  every minibuffer without resetting it, so it kept growing. Now each
  minibuffer starts from the database threshold.
- When a channel was first added to the pulse map, the whole pulse_vec
  was inserted and later pieces were appended again (duplicates for
  channels with more than one waveform piece). Now only the current
  piece is inserted.
…ble, reject single-board events"

- queryNearestACCID: log an error and stop the toolchain if no ACC ID is found for the LAPPD manufacturer ID.
- Set NewLAPPDDataAvailable only when the event was actually saved, and reset it at the start of each Execute().
- Reject events with an uneven number of boards.
- Clear LAPPDLoadedTimeStamps in CleanDataObjects().
@anuj-guptta

Copy link
Copy Markdown
Contributor Author

This PR also includes a small LAPPDLoadStore fix so the build passes. The same fix is already in the dual-ACDC event building PR, I hope that is not an issue.

@S81D

S81D commented Oct 15, 2026

Copy link
Copy Markdown
Collaborator

Looks good to me

@marc1uk
marc1uk merged commit 62f43dd into ANNIEsoft:Application Oct 21, 2026
1 check passed
@anuj-guptta
anuj-guptta deleted the claude-fixes-2 branch October 23, 2026 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants