Micromegas decoder update by hupereir · Pull Request #4297 · sPHENIX-Collaboration/coresoftware

hupereir · 2026-06-11T19:22:01Z

This is TPOT fix for overlapping time frames, similar to #4292
TPOT decoder still uses taggers (GTM and enddat) to associate FEE data to 40bits gtm BCO, unlike the TPC, but this is really an orthogonal change.

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work for users)
Requiring change in macros repository (Please provide links to the macros pull request in the last section)
I am a member of GitHub organization of sPHENIX Collaboration, EIC, or ECCE (contact Chris Pinkenburg to join)

What kind of change does this PR introduce? (Bug fix, feature, ...)

TODOs (if applicable)

Links to other PRs in macros and calibration repositories (if applicable)

Micromegas Decoder Update: Overlapping Time Frames Fix

Motivation / context

Fix overlapping time-frame handling in the Micromegas (TPOT) decoder so FEE data aligned to GTM 40-bit BCOs are correctly associated and truncated/overlapping waveforms are recovered. Change mirrors approach used in the recent TPC time-frame work (PR 4292) while keeping the existing GTM/enddat tagging behavior for mapping FEE data to GTM BCOs.

Key changes

Introduced signed/unsigned BCO difference utilities with explicit GTM/FEE rollover handling and set m_max_fee_sync_time = 1024*32.
Reworked BCO reference representation to a matched pair (fee_bco, gtm_bco) and added accessor get_bco_matching_reference().
Overhauled BCO matching logic and multiplier adjustment to use signed GTM diffs and unsigned fee-diff comparisons.
Changed FillPool API to accept a target BCO (FillPool(uint64_t target_bco)); removed GetSomeMoreEvents() and made SetBcoPoolSize() a no-op.
Refactored per-FEE state: raw-hit storage moved to per-FEE maps/arrays; per-FEE packet-ID mapping made explicit; removed older global BCO/stack containers.
Added recover_truncated_waveforms(target_bco) to detect and recover truncated/overlapping waveforms by creating shifted copies of hits and re-registering them; FillPool now drives reading by is_more_data_required(target_bco) and calls the recovery step.
MicromegasRawHitv3: added const accessor get_adc_waveforms() that exposes waveform pairs; fixed a warning-message typo.
Minor API/implementation cleanups: deprecated ClearCurrentEvent behavior, simplified Print(), used unique_ptr when creating recovered hits, and various formatting/lint fixes.

Potential risk areas

Reconstruction behavior: Core BCO-matching algorithm and multiplier logic changed — verification with Run-3/representative data is required to ensure no mis-associations.
IO / object exposure: MicromegasRawHitv3 now exposes waveform pair structure (start_time, samples) — downstream code expecting different views may need updates.
Performance: recover_truncated_waveforms() performs per-FEE scans and may add CPU overhead for high-occupancy events.
Thread-safety / concurrency: Per-FEE fixed arrays and internal maps change ownership/layout; concurrent access patterns must be reviewed if these objects are used from multiple threads.
Integration: Changes touch streaming input manager, BCO matching class, and raw-hit formats — potential cascading effects across decoding and reconstruction stages.

Possible future improvements

Consolidate and simplify recover_truncated_waveforms() implementation and reduce code duplication.
Benchmark and optimize waveform-recovery scanning for high-rate conditions.
Consider exposing a stable, small API for waveform access to avoid breaking callers that expect flat sample arrays.
Generalize BCO-difference utilities for reuse across other detector decoders.

Note: AI-generated summaries can be incorrect or incomplete. Review the changed files (particularly BCO matching, FillPool/recovery logic, and the raw-hit interface) carefully before approving for production.

…ors accodringly

check the difference between the requested BCO and the last BCO found in the TPOT data stream, including some extra delay corresponding to how much time it takes for TPOT to send all the data for said trigger.

- also store per FEE packet id.

- do not save hits for which there is no waveforms.

…rollover. Based on Jin's code.

coderabbitai · 2026-06-11T19:22:39Z

Warning

Review limit reached

@hupereir, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 41 minutes and 25 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more credits in the billing tab to continue.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 06b4e612-7e8b-4540-8485-42313eaa4ede

📥 Commits

Reviewing files that changed from the base of the PR and between ad81fd1 and 820ad8a.

📒 Files selected for processing (1)

offline/framework/fun4allraw/MicromegasBcoMatchingInformation_v2.cc

📝 Walkthrough

Walkthrough

This PR refactors Micromegas BCO matching and raw-hit processing to use rollover-aware signed/unsigned clock-difference utilities, restructure internal state from separate "first" fields to a paired reference type, and change FillPool from event-count-based to target-BCO-driven control flow with per-FEE waveform recovery.

Changes

BCO Matching and Raw-Hit Refactor

Layer / File(s)	Summary
Raw hit waveform accessor and typo fix `offline/framework/ffarawobjects/MicromegasRawHitv3.cc`, `offline/framework/ffarawobjects/MicromegasRawHitv3.h`	New public `get_adc_waveforms()` const accessor exposes the internal waveform vector, and a constructor warning message typo ("moethod" → "method") is corrected.
BCO clock difference utilities and matching pair type `offline/framework/fun4allraw/MicromegasBcoMatchingInformation_v2.h`, `offline/framework/fun4allraw/MicromegasBcoMatchingInformation_v2.cc`	Adds public type alias `m_bco_matching_pair_t`, static methods for signed/unsigned GTM/FEE BCO differences with ±half-range rollover handling, configuration setter for `m_max_fee_sync_time` (1024*32), and internal clock-bit/mask constants used for diff calculations.
BCO matching computation refactor using new diff utilities `offline/framework/fun4allraw/MicromegasBcoMatchingInformation_v2.cc`	Predicted fee BCO now uses `get_signed_gtm_bco_diff`; `is_more_data_required` compares signed differences against sync-time bounds; reference init and verification switch from separate "first" fields to pair assignments; `find_gtm_bco` and `update_multiplier_adjustment` use new signed/unsigned fee-difference helpers for matching and tolerance checks throughout.
SingleMicromegasPoolInput header restructuring `offline/framework/fun4allraw/SingleMicromegasPoolInput_v2.h`	`FillPool` signature changes from `nevents` count to `target_bco` parameter; `GetSomeMoreEvents()` removed; `SetBcoPoolSize` becomes obsolete no-op; constants expanded with `MAX_FEECHANNELCOUNT`; fee data buffer changes to fixed `std::array`; raw-hit storage refactored to per-FEE maps; prior FEE/BCO tracking state (`m_FEEBclkMap`, `m_BclkStack`) removed.
SingleMicromegasPoolInput implementation refactor `offline/framework/fun4allraw/SingleMicromegasPoolInput_v2.cc`	`FillPool` loop now terminates on `is_more_data_required(target_bco)` predicate and invokes `recover_truncated_waveforms`; `Print` and `CleanupUsedPackets` simplified for per-FEE storage; `ClearCurrentEvent` replaced with deprecation notice; FEE decoding records packet IDs and uses matching-reference accessor; `process_fee_data` uses new raw-hit map storage; new `recover_truncated_waveforms` method finds matching/overlapping BCOs and restores truncated waveforms via shifted hit copies with moved ADC samples.
Streaming manager integration with target BCO `offline/framework/fun4allraw/Fun4AllStreamingInputManager.cc`	`FillMicromegasPool()` computes `ref_bco_minus_range` from reference BCO and negative BCO range, passes it to each input's `FillPool(target_bco)` call; includes minor formatting adjustments in INTT and MVTX methods.

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

hupereir · 2026-06-11T19:23:17Z

coderabbitai

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

offline/framework/fun4allraw/SingleMicromegasPoolInput_v2.cc (1)
964-990: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Do not publish zero-waveform hits.

This path always allocates and registers a MicromegasRawHit, even if waveform extraction produced no ADC segments. That creates hits with no payload in both the streaming manager and the per-FEE cache, which is the same invalid state the recovery path already filters out.
Suggested fix
-    // create new hit
+    if (payload.waveforms.empty())
+    {
+      continue;
+    }
+
+    // create new hit
     auto newhit = std::make_unique<MicromegasRawHit_impl>();

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: a6bf1576-7fe2-427d-814d-4c545b5f1eaf

📥 Commits

Reviewing files that changed from the base of the PR and between 0ffd526 and a2a8dfb.

📒 Files selected for processing (7)

offline/framework/ffarawobjects/MicromegasRawHitv3.cc
offline/framework/ffarawobjects/MicromegasRawHitv3.h
offline/framework/fun4allraw/Fun4AllStreamingInputManager.cc
offline/framework/fun4allraw/MicromegasBcoMatchingInformation_v2.cc
offline/framework/fun4allraw/MicromegasBcoMatchingInformation_v2.h
offline/framework/fun4allraw/SingleMicromegasPoolInput_v2.cc
offline/framework/fun4allraw/SingleMicromegasPoolInput_v2.h

sphenix-jenkins-ci · 2026-06-12T02:38:40Z

Build & test report

Report for commit a2a8dfbcd55646e696c2446241acc63d192faa92:

builds and tests overall are FAILURE.
Build with configuration of alma9.2-gcc-14.2.0 / clang is SUCCESS, 📊clang report (full)/(new), build log
Build with configuration of alma9.2-gcc-14.2.0 / new is SUCCESS, 📊Compiler report (full)/(new), build log
- Generating DST and readback: build is SUCCESS
- Calorimeter QA: build is SUCCESS, 📊 trends
  - 📊 QA-calorimeter for e- at p_T=4GeV : combined Chi2/nDoF = 27.45 / 72, and combined p-Value = 1
  - 📊 QA-calorimeter for pi+ at p_T=30GeV : combined Chi2/nDoF = 41.7331 / 72, and combined p-Value = 0.99837
  - 📊 QA-calorimetric-jet for e- at p_T=4GeV : combined Chi2/nDoF = 0.4559 / 42, and combined p-Value = 1
  - 📊 QA-calorimetric-jet for pi+ at p_T=30GeV : combined Chi2/nDoF = 14.2531 / 42, and combined p-Value = 0.999981
- Tracking QA with Distortions: build is SUCCESS, 📊 trends
  - 📊 QA-distortions
- Tracking QA at low occupancy: build is SUCCESS, 📊 trends
  - 📊 QA-Intt : combined Chi2/nDoF = -0 / 72, and combined p-Value = 1
  - 📊 QA-Micromegas : combined Chi2/nDoF = -0 / 20, and combined p-Value = 1
  - 📊 QA-Mvtx : combined Chi2/nDoF = -0 / 54, and combined p-Value = 1
  - 📊 QA-Tpc : combined Chi2/nDoF = -0 / 56, and combined p-Value = 1
  - 📊 QA-tracking : combined Chi2/nDoF = 15.7576 / 56, and combined p-Value = 1
  - 📊 QA-vertexing : combined Chi2/nDoF = 39.0875 / 112, and combined p-Value = 1
- Tracking QA for Pythia D0-jet: build is SUCCESS, 📊 trends
  - 📊 QA-Intt : combined Chi2/nDoF = -0 / 72, and combined p-Value = 1
  - 📊 QA-Micromegas : combined Chi2/nDoF = -0 / 20, and combined p-Value = 1
  - 📊 QA-Mvtx : combined Chi2/nDoF = -0 / 54, and combined p-Value = 1
  - 📊 QA-Tpc : combined Chi2/nDoF = -0 / 56, and combined p-Value = 1
  - 📊 QA-tracking : combined Chi2/nDoF = 6.36278 / 50, and combined p-Value = 1
  - 📊 QA-vertexing : combined Chi2/nDoF = 37.0285 / 112, and combined p-Value = 1
- Tracking QA from run2pp 53877: build is SUCCESS, 📊 trends
  - 📊 QA-intt : combined Chi2/nDoF = -0 / 18, and combined p-Value = 1
  - 📊 QA-micromegas : combined Chi2/nDoF = 1.56293 / 6, and combined p-Value = 0.955205
  - 📊 QA-mvtx : combined Chi2/nDoF = -0 / 18, and combined p-Value = 1
  - 📊 QA-siliconseeds : combined Chi2/nDoF = -0 / 20, and combined p-Value = 1
  - 📊 QA-tpc : combined Chi2/nDoF = -0 / 48, and combined p-Value = 1
  - 📊 QA-tpcseeds : combined Chi2/nDoF = -0 / 28, and combined p-Value = 1
  - 📊 QA-trackfit : combined Chi2/nDoF = -0 / 32, and combined p-Value = 1
- system alma9.2-gcc-14.2.0, build new: run the default sPHENIX macro: build is SUCCESS, output, trends 📊
- system alma9.2-gcc-14.2.0, build new: run the default CaloProduction/Fun4All_Year2.C macro: build is SUCCESS, output, trends 📊
- system alma9.2-gcc-14.2.0, build new: run the default StreamingProduction/Fun4All_Stream_Combiner.C macro: build is SUCCESS, output, trends 📊
- system alma9.2-gcc-14.2.0, build new: Valgrind test of CaloProduction/Fun4All_Year2.C: build is SUCCESS, 📊valgrind report, trends 📊
- system alma9.2-gcc-14.2.0, build new: Valgrind test of StreamingProduction/Fun4All_Stream_Combiner.C: build is SUCCESS, 📊valgrind report, trends 📊
- system alma9.2-gcc-14.2.0, build new: run the overlap check for sPHENIX macro: build is SUCCESS, output
- system alma9.2-gcc-14.2.0, build new: Valgrind test: build is UNSTABLE, 📊valgrind report, trends 📊
Build with configuration of alma9.2-gcc-14.2.0 / scan is SUCCESS, 📊scan-build report (full)/(new), build log
clang-tidy is FAILURE, 📊clang-tidy report (full)/(new)
cpp-check is SUCCESS, 📊cppcheck report (full)/(new)

Automatically generated by sPHENIX Jenkins continuous integration

hupereir added 10 commits June 11, 2026 15:15

store reference as a pair, (as in the matching map) and update access…

9296314

…ors accodringly

Change the pool filling strategy so that it matches the TPC better:

f79886f

check the difference between the requested BCO and the last BCO found in the TPOT data stream, including some extra delay corresponding to how much time it takes for TPOT to send all the data for said trigger.

fixed error message

5afdda3

- implement per FEE local rawhit storage

22c5f1c

- also store per FEE packet id.

First shot at recovering overlaping trigger frames.

316fd67

make sure duplicated waveform don't extend beyond the 1024 limit.

b169597

- Fix truncating raw hits.

36310d9

- do not save hits for which there is no waveforms.

sanitize BCO differences (GTM and FEE) to properly account for clock …

3a1f3f8

…rollover. Based on Jin's code.

fixed some integer type

6dec602

removed comment

a2a8dfb

coderabbitai Bot reviewed Jun 11, 2026

View reviewed changes

hupereir added 3 commits June 11, 2026 19:39

clang-tidy

606cd93

use unique_ptr for creating new hits during truncated waveform recovery.

ad81fd1

bettern handling of rollover when doing gtm bco comparisons.

820ad8a

hupereir marked this pull request as draft June 12, 2026 00:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Micromegas decoder update#4297

Micromegas decoder update#4297
hupereir wants to merge 13 commits into
sPHENIX-Collaboration:masterfrom
hupereir:micromegas-decoder-update

hupereir commented Jun 11, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jun 11, 2026 •

edited

Loading

Review limit reached

Walkthrough

Changes

Uh oh!

hupereir commented Jun 11, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sphenix-jenkins-ci Bot commented Jun 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

hupereir commented Jun 11, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Types of changes

What kind of change does this PR introduce? (Bug fix, feature, ...)

TODOs (if applicable)

Links to other PRs in macros and calibration repositories (if applicable)

Micromegas Decoder Update: Overlapping Time Frames Fix

Motivation / context

Key changes

Potential risk areas

Possible future improvements

Uh oh!

coderabbitai Bot commented Jun 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Walkthrough

Changes

Uh oh!

hupereir commented Jun 11, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sphenix-jenkins-ci Bot commented Jun 12, 2026

Build & test report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

hupereir commented Jun 11, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 11, 2026 •

edited

Loading