Skip to content

feat: fee masking in tpc decoder#4227

Merged
osbornjd merged 7 commits into
sPHENIX-Collaboration:masterfrom
osbornjd:timeframebuilder_badfees
Mar 19, 2026
Merged

feat: fee masking in tpc decoder#4227
osbornjd merged 7 commits into
sPHENIX-Collaboration:masterfrom
osbornjd:timeframebuilder_badfees

Conversation

@osbornjd

@osbornjd osbornjd commented Mar 18, 2026

Copy link
Copy Markdown
Contributor

This PR adds the ability to read from a CDB file responsible for masking bad FEEs on a run by run basis into the tpc decoder. The files need to be committed to the CDB, so in the meantime if no file is found the function does nothing.

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)

FEE Masking in TPC Decoder

Motivation

This PR adds runtime capability to mask (skip processing of) Front-End Electronics (FEEs) identified as problematic on a run-by-run basis. The implementation reads FEE mask information from the Calibration Database (CDB) and applies these masks during TPC data decoding. If no CDB file is available (e.g., during development before calibration commits), the masking is a no-op, ensuring graceful fallback behavior.

Key Changes

  • Build system: Extended libfun4allraw library dependencies to include ffamodules and cdbobjects libraries (Makefile.am)
  • TpcTimeFrameBuilder class:
    • Added fillBadFeeMap() public method to initialize bad-FEE mapping from CDB at runtime
    • Reads TPC_DECODER_BAD_FEE CDB URL and loads corresponding CDBTTree calibration data
    • Populates internal m_maskedFEEs map (mapping packet_id to set of masked FEE IDs)
    • Added masking logic to skip processing for any FEE in the masked set during data processing
  • SingleTpcTimeFrameInput: Calls fillBadFeeMap() immediately after creating each TpcTimeFrameBuilder instance
  • Minor formatting: Style/whitespace adjustments in header files

Potential Risk Areas

  • CDB dependency: Decoder now depends on external CDB availability; behavior change if CDB access fails or returns unexpected data format
  • Performance: Additional map lookups added to hot processing path (per-FEE processing loop); impact depends on map size and access patterns
  • Data consistency: Run-by-run masking could cause variations in reconstruction if CDB state drifts; recommend validation that masked FEEs are consistent across processing

Possible Future Improvements

  • Add monitoring/logging of which FEEs are masked per run for validation
  • Consider caching CDB data if multiple decoders are instantiated
  • Add optional debugging output showing statistics on masked FEE hits

Note: This summary is AI-generated and should be validated by domain experts familiar with the TPC decoder and CDB infrastructure. AI may have misinterpreted technical details or missed nuances in the implementation.

@osbornjd osbornjd requested a review from blackcathj March 18, 2026 18:46
@coderabbitai

coderabbitai Bot commented Mar 18, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

The changes implement a bad-FEE (Front-End Electronics) masking system for TPC data processing. TpcTimeFrameBuilder now reads bad-FEE calibration data from the conditions database, populates an internal mask, and skips processing of masked FEEs during time-frame construction. The system is initialized via a new fillBadFeeMap() method called during instantiation.

Changes

Cohort / File(s) Summary
Build System
offline/framework/fun4allraw/Makefile.am
Extended libfun4allraw_la_LIBADD with -lffamodules and -lcdbobjects to support calibration database integration.
TPC TimeFrame Builder Implementation
offline/framework/fun4allraw/TpcTimeFrameBuilder.cc, offline/framework/fun4allraw/TpcTimeFrameBuilder.h
Introduced fillBadFeeMap() public method that reads bad-FEE mappings from CDB calibration data. Added logic to skip FEE processing based on per-packet-id masks stored in m_maskedFEEs. Added CDBTTree and CDBInterface includes.
TPC TimeFrame Input
offline/framework/fun4allraw/SingleTpcTimeFrameInput.cc, offline/framework/fun4allraw/SingleTpcTimeFrameInput.h
Integrated fillBadFeeMap() call after TpcTimeFrameBuilder instantiation. Minor whitespace/formatting adjustments in header.

Sequence Diagram(s)

sequenceDiagram
    participant STFI as SingleTpcTimeFrameInput
    participant TTFB as TpcTimeFrameBuilder
    participant CDB as CDBInterface
    participant CAL as CDBTTree<br/>(Calibration)
    
    STFI->>TTFB: new TpcTimeFrameBuilder(packet_id)
    STFI->>TTFB: fillBadFeeMap()
    activate TTFB
    TTFB->>CDB: read URL for TPC_DECODER_BAD_FEE
    CDB-->>TTFB: calibration file path
    TTFB->>CAL: load CDBTTree calibration
    CAL-->>TTFB: N_MASKED_FEES & EBDC→FEEID mappings
    TTFB->>TTFB: populate m_maskedFEEs[packet_id_derived]
    deactivate TTFB
    Note over TTFB: During FEE processing:<br/>skip if FEE_ID in m_maskedFEEs
Loading
📝 Coding Plan
  • Generate coding plan for human review comments

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.

Tip

CodeRabbit can generate a title for your PR based on the changes.

Add @coderabbitai placeholder anywhere in the title of your PR and CodeRabbit will replace it with a title based on the changes in the PR. You can change the placeholder by changing the reviews.auto_title_placeholder setting.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

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/SingleTpcTimeFrameInput.cc (1)

294-301: ⚠️ Potential issue | 🟠 Major

Reinitialize bad-FEE masks on run/file transitions, not only on first builder creation.

The call is currently one-time per builder. Since builders persist in m_TpcTimeFrameBuilderMap, subsequent runs/files can reuse stale masking state and apply the wrong mask set.

Proposed fix direction
     if (RunNumber() == 0)
     {
       RunNumber(evt->getRunNumber());
+      for (auto& [id, builder] : m_TpcTimeFrameBuilderMap)
+      {
+        builder->fillBadFeeMap();
+      }

       if (Verbosity() >= 1)
       {
         std::cout << __PRETTY_FUNCTION__ << ": Fetching new file " << FileName()

As per coding guidelines, **/*.{cc,cpp,cxx,c}: “Prioritize correctness, memory safety, error handling, and thread-safety.”


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 80d1988d-20a8-4e2b-9550-da5a7b97fb9d

📥 Commits

Reviewing files that changed from the base of the PR and between d3f5d82 and f947ccd.

📒 Files selected for processing (5)
  • offline/framework/fun4allraw/Makefile.am
  • offline/framework/fun4allraw/SingleTpcTimeFrameInput.cc
  • offline/framework/fun4allraw/SingleTpcTimeFrameInput.h
  • offline/framework/fun4allraw/TpcTimeFrameBuilder.cc
  • offline/framework/fun4allraw/TpcTimeFrameBuilder.h

Comment on lines +2069 to +2090
void TpcTimeFrameBuilder::fillBadFeeMap()
{
const std::string filename = CDBInterface::instance()->getUrl("TPC_DECODER_BAD_FEE");

if (filename.empty())
{
if (m_verbosity > 0)
{
std::cout << "TpcTimeFrameBuilder::fillBadFeeMap - no file found for TPC_DECODER_BAD_FEE, not filling bad fee map" << std::endl;
}
return;
}

CDBTTree cdbtree(filename);
cdbtree.LoadCalibrations();

const int nentries = cdbtree.GetSingleIntValue("N_MASKED_FEES");

for (int i = 0; i < nentries; i++)
{
m_maskedFEEs[cdbtree.GetIntValue(i, "EBDC")].insert(cdbtree.GetIntValue(i, "FEEID"));
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Reset mask state before loading CDB data.

fillBadFeeMap() only inserts into m_maskedFEEs. If this method is called again (new run/file context), previous masks remain; and when no CDB file is found, old masks still stay active. That can incorrectly suppress good FEEs.

Proposed fix
 void TpcTimeFrameBuilder::fillBadFeeMap()
 {
+  // Never carry mask state across CDB refreshes.
+  m_maskedFEEs.clear();
+
   const std::string filename = CDBInterface::instance()->getUrl("TPC_DECODER_BAD_FEE");

   if (filename.empty())
   {
     if (m_verbosity > 0)
     {
       std::cout << "TpcTimeFrameBuilder::fillBadFeeMap - no file found for TPC_DECODER_BAD_FEE, not filling bad fee map" << std::endl;
     }
     return;
   }

As per coding guidelines, **/*.{cc,cpp,cxx,c}: “Prioritize correctness, memory safety, error handling, and thread-safety.”

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
void TpcTimeFrameBuilder::fillBadFeeMap()
{
const std::string filename = CDBInterface::instance()->getUrl("TPC_DECODER_BAD_FEE");
if (filename.empty())
{
if (m_verbosity > 0)
{
std::cout << "TpcTimeFrameBuilder::fillBadFeeMap - no file found for TPC_DECODER_BAD_FEE, not filling bad fee map" << std::endl;
}
return;
}
CDBTTree cdbtree(filename);
cdbtree.LoadCalibrations();
const int nentries = cdbtree.GetSingleIntValue("N_MASKED_FEES");
for (int i = 0; i < nentries; i++)
{
m_maskedFEEs[cdbtree.GetIntValue(i, "EBDC")].insert(cdbtree.GetIntValue(i, "FEEID"));
}
void TpcTimeFrameBuilder::fillBadFeeMap()
{
// Never carry mask state across CDB refreshes.
m_maskedFEEs.clear();
const std::string filename = CDBInterface::instance()->getUrl("TPC_DECODER_BAD_FEE");
if (filename.empty())
{
if (m_verbosity > 0)
{
std::cout << "TpcTimeFrameBuilder::fillBadFeeMap - no file found for TPC_DECODER_BAD_FEE, not filling bad fee map" << std::endl;
}
return;
}
CDBTTree cdbtree(filename);
cdbtree.LoadCalibrations();
const int nentries = cdbtree.GetSingleIntValue("N_MASKED_FEES");
for (int i = 0; i < nentries; i++)
{
m_maskedFEEs[cdbtree.GetIntValue(i, "EBDC")].insert(cdbtree.GetIntValue(i, "FEEID"));
}

@sphenix-jenkins-ci

Copy link
Copy Markdown

Build & test report

Report for commit f947ccde161a826340a055b40dc4a39e8b5e5fd2:
Jenkins on fire


Automatically generated by sPHENIX Jenkins continuous integration
sPHENIX             jenkins.io

@sphenix-jenkins-ci

Copy link
Copy Markdown

Build & test report

Report for commit cea59d00c328005edb502f46033076e322a69f82:
Jenkins on fire


Automatically generated by sPHENIX Jenkins continuous integration
sPHENIX             jenkins.io

@osbornjd

Copy link
Copy Markdown
Contributor Author

This clang-tidy warning is not related to this PR

@osbornjd osbornjd merged commit 7c6b32f into sPHENIX-Collaboration:master Mar 19, 2026
19 of 22 checks passed
@osbornjd osbornjd deleted the timeframebuilder_badfees branch March 19, 2026 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant