Rewritten Run-3 TPC event builder for the new Run3 TPC firmware#4292
Merged
osbornjd merged 35 commits intoJun 11, 2026
Conversation
…e into TPCTimeFrameBuilder5
…e into TPCTimeFrameBuilder5
…nd update fill methods
…e into TPCTimeFrameBuilder5
…e into TPCTimeFrameBuilder6
Contributor
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. 💤 Files selected but had no reviewable changes (1)
⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds a TpcTimeFrameBuilderBase interface; refactors TpcTimeFrameBuilder to inherit and override it; introduces TpcTimeFrameBuilderRun3 with Run3 parsing/BCO matching; updates SingleTpcTimeFrameInput to select builders at runtime, propagate FillPool status, and adds TpcRawHitv3 ADC waveform accessors. ChangesTPC Time-Frame Builder Polymorphism
Possibly Related PRs
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f8a19664-e6f1-4a8e-bd1b-fac9da58e06c
📒 Files selected for processing (10)
offline/framework/ffarawobjects/TpcRawHitv3.hoffline/framework/fun4allraw/Fun4AllStreamingInputManager.ccoffline/framework/fun4allraw/Makefile.amoffline/framework/fun4allraw/SingleStreamingInput.hoffline/framework/fun4allraw/SingleTpcTimeFrameInput.ccoffline/framework/fun4allraw/SingleTpcTimeFrameInput.hoffline/framework/fun4allraw/TpcTimeFrameBuilder.hoffline/framework/fun4allraw/TpcTimeFrameBuilderBase.hoffline/framework/fun4allraw/TpcTimeFrameBuilderRun3.ccoffline/framework/fun4allraw/TpcTimeFrameBuilderRun3.h
Member
Author
|
clang-tidy errors are from header file mismatch in nightly builds |
Build & test reportReport for commit c2e3c3aef1b18971f2e40e52921c1889a2d2eff6:
Automatically generated by sPHENIX Jenkins continuous integration |
Build & test reportReport for commit c53fbbf0e43a56c7d184cc3e092d7e0163766bce:
Automatically generated by sPHENIX Jenkins continuous integration |
…pt taking streaming data
…cleanup functions
Member
Author
|
Latest patches handle a rare operation scenario that GL1 trigger is paused for a long time while TPC kept taking streaming data. In the past this lead to increased memory usage to ~10GB to buffer the hits in searching for the next trigger in TPC. The new patch drop hits unless within 7ms of the trigger (1/4 turn of FEE clock rollover) to mitigate memory usage. |
Contributor
Christof's latest QA check suggests that the improvement can also be verified in the downstream clusterizer
|
osbornjd
approved these changes
Jun 11, 2026
osbornjd
left a comment
osbornjd
left a comment
Contributor
There was a problem hiding this comment.
After some testing I believe this is ready to go in, and clang tidy warnings should be addressed. The errors are spurious
Build & test reportReport for commit f57ffffae66eb256056101a3a7f561cb6378ce22:
Automatically generated by sPHENIX Jenkins continuous integration |
Build & test reportReport for commit 204e425afa9e1a9ea15bb61d32613e6d60329c18:
Automatically generated by sPHENIX Jenkins continuous integration |
osbornjd
merged commit 0ffd526
into
sPHENIX-Collaboration:master
19 of 22 checks passed
Open
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Types of changes
What kind of change does this PR introduce? (Bug fix, feature, ...)
This is a rewritten Run-3 TPC timeframe and event builder for the Run3 TPC firmware (
IDTPCFEEV5/IDTPCFEEV6packets) while preserving the existingTpcTimeFrameBuilderbehavior for legacy Run2 firmware (IDTPCFEEV4). Run3 has a deterministic clock frequency, locking FEE to FELIX clock. By separating Run3 event building from Run2, it allows for a clean event building with a deterministic association of FEE waveforms to GL1 trigger.This PR also implement another long planned feature to rebuild/add the SAMPA
RawHitat event building stage to recover full-time-length SAMPA waveforms that was truncated by the next trigger if it comes with in 100us in spacing. This is needed to allow for use all statistics for the extended streaming window, allowing collecting the entire TPC drift window which during data taking is truncated to be associated to the next event trigger.List of main changes:
TpcTimeFrameBuilderRun3with exact integer GTM-to-FEE clock matching using the30/8clock ratio and 20-bit FEE rollover handling.SingleTpcTimeFrameInputto automatically select for the new event builder for Run3AI usage disclosure
The agentic AI tool Codex was used to write majority of the code under the "goal mode", in which specific instruction were given to codex on how to perform event building then Codex implemented it in code and run code checker/performance bench marker.
The cost of both writing and testing was about 7M input tokens and 0.5M output tokens. Codex speed up code writing by about x10 and debugging by about x2.
Validations
Tested code checker and performance check with https://github.com/blackcathj/macros/blob/f8f7e2ed6b61233531bd7fdb4e13e32f8f8372d6/StreamingProduction/Fun4All_TPC_SingleStream_Combiner.C
output QA ROOT histograms.
The QA plot generated with 0.5M events are summarized in this notebook :
https://nbviewer.sphenix.bnl.gov/github/blackcathj/macros/blob/f8f7e2ed6b61233531bd7fdb4e13e32f8f8372d6/StreamingProduction/qa_plot.ipynb
In particular, it shows a sharp coincidence peak within 1 RHIC BCO which 4 FEE clock (40Mhz)
It also shows the expected second trigger truncation (1st plot) and after the truncation recovery refilling of the ADC bins (2nd plot)
Callgrind shows the new Run-3 QA/cache/fill path is a small fraction of runtime
The dominating part is still CRC calculation + DST ROOT compression
Massif memory usage is stable and below 2GB
TODO
Summary
Motivation / Context
Enable correct TPC timeframe/event building for Run‑3 firmware (IDTPCFEEV5/IDTPCFEEV6) while preserving Run‑2 behavior (IDTPCFEEV4). Provide exact GTM↔FEE clock alignment and recover full SAMPA waveforms truncated by nearby triggers so the full TPC drift window is available for reconstruction and QA.
Key changes
Potential risk areas
Possible future improvements
Note on AI assistance: portions of the implementation and tests were developed with AI assistance (Codex). AI can be mistaken—review clock‑matching, BCO‑rollover handling, parsing and waveform‑recovery logic carefully and validate with independent tests.