Skip to content

Comments

bypass calibration existence check during calibration passes#4180

Merged
pinkenburg merged 1 commit intosPHENIX-Collaboration:masterfrom
mchiu-bnl:mbd
Feb 18, 2026
Merged

bypass calibration existence check during calibration passes#4180
pinkenburg merged 1 commit intosPHENIX-Collaboration:masterfrom
mchiu-bnl:mbd

Conversation

@mchiu-bnl
Copy link
Contributor

@mchiu-bnl mchiu-bnl commented Feb 17, 2026

comment: modifying the reco to abort if a mbd calibration doesn't exist broke the mbd calibration, just fixing it back. Also added a method to return MbdEvent so one can easily access the waveform and other reco debug info from an analysis module.

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)

Motivation / Context

A recent change to MBD reconstruction that aborts processing when calibration data is missing inadvertently broke the MBD calibration workflow itself. The calibration passes need to process events even when pre-existing calibration data is unavailable. This PR restores the previous behavior while maintaining error handling for normal data reconstruction.

Key Changes

  • Calibration existence check bypass during calibration runs (MbdEvent.cc): Modified the abort condition to only trigger when calibration data is missing (status < 0) and processing is in normal mode (_calpass == 0). This allows calibration passes (_calpass > 0) to proceed without pre-existing calibration constants, enabling the multi-pass calibration workflow.

  • New MbdEvent accessor method (MbdReco.h): Added public GetMbdEvent() method that returns a raw pointer to the internal MbdEvent object, enabling analysis modules convenient access to raw waveform data and reconstruction debug information without copying data.

  • Debug statement cleanup (MbdSig.cc): Disabled verbose debug output for channel 9 pulse information in the SetXY() function.

Potential Risk Areas

  • Reconstruction behavior change: The modified abort logic may allow corrupted or missing calibration data to propagate through normal processing (non-calibration runs) if the status check is not properly triggered. The condition status < 0 && _calpass==0 should be verified to ensure it correctly distinguishes between calibration and production runs.

  • API exposure: The new GetMbdEvent() method returns a raw pointer, which could enable external code to modify internal state without proper checks. Users should treat this as read-only access.

  • Multi-pass calibration workflow: Verify that the calibration passes (calpass=1,2,3) properly progress through their intended sequence without data quality issues.

Notes

AI note: Please verify the actual values and semantics of _calpass (especially the distinction between 0=normal processing vs. 1,2,3=calibration passes) and confirm that the status codes from Download_All() are correctly handled across different reconstruction modes.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 17, 2026

No actionable comments were generated in the recent review. 🎉


📝 Walkthrough

Walkthrough

This PR makes three targeted changes to the MBD subsystem: adjusts error-handling logic in MbdEvent initialization to distinguish normal processing from calibration modes, adds a public accessor method to MbdReco for accessing the internal MbdEvent instance, and disables debug output in MbdSig's verbose block.

Changes

Cohort / File(s) Summary
MbdEvent Initialization Logic
offline/packages/mbd/MbdEvent.cc
Modified abort condition after Download_All in InitRun from status == -1 to status < 0 && _calpass == 0, allowing negative status values during non-normal calibration flows without aborting.
MbdReco API Expansion
offline/packages/mbd/MbdReco.h
Added public accessor method GetMbdEvent() that returns a raw pointer to the internal m_mbdevent unique_ptr, exposing the MbdEvent instance without ownership transfer.
MbdSig Debug Output
offline/packages/mbd/MbdSig.cc
Disabled verbose debug-printing block in SetXY function (channel 9 raw pulse output) by wrapping it in a comment block.

Possibly related PRs

✨ Finishing touches
  • 📝 Generate docstrings

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.


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.

@sphenix-jenkins-ci
Copy link

Build & test report

Report for commit 388a36bd33b650b008df3a3f56c70d3ec345bb9c:
Jenkins passed


Automatically generated by sPHENIX Jenkins continuous integration
sPHENIX             jenkins.io

@pinkenburg
Copy link
Contributor

clang-tidy errors are from another PR

@pinkenburg pinkenburg merged commit f464d02 into sPHENIX-Collaboration:master Feb 18, 2026
21 of 22 checks passed
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.

2 participants