Skip to content

Undo checks for existing calibs#4151

Merged
pinkenburg merged 2 commits intosPHENIX-Collaboration:masterfrom
mchiu-bnl:mbd
Feb 1, 2026
Merged

Undo checks for existing calibs#4151
pinkenburg merged 2 commits intosPHENIX-Collaboration:masterfrom
mchiu-bnl:mbd

Conversation

@mchiu-bnl
Copy link
Contributor

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

comment: Undo change to abort if sampmax and ped calibs are missing.

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)

PR Summary: Undo checks for existing calibrations

Motivation/Context

The MBD (Barrel Barrel Detector) calibration system was previously configured to abort execution if critical calibration files (sampmax and pedestal) were missing from the database or file system. This hard-fail behavior prevented the possibility of calculating these calibrations on the fly during reconstruction, limiting flexibility in data processing workflows.

Key Changes

  • Removed early-exit checks in Download_All(): Deleted validation logic that returned error status (-1) when MBD_SAMPMAX or MBD_PED URLs were empty strings
  • Relaxed error handling in Download_Ped(): Changed the behavior when pedestal calibration data is not found from returning error status to logging a warning and continuing execution (calibration values remain as NaN)
  • Relaxed error handling in Download_SampMax(): Changed the behavior when sampmax calibration data is not found from returning error status to logging a warning and continuing execution (calibration values remain as -1)
  • Code size: 14 lines removed from MbdCalib.cc

Reconstruction Behavior Changes

This change enables the calibration system to gracefully handle missing calibrations instead of aborting:

  • Missing sampmax and pedestal calibrations no longer halt event processing
  • Downstream code can now attempt to calculate these calibrations algorithmically or use default/fallback values
  • Diagnostics are still logged (warnings printed) when calibrations are missing
  • Code continues with uninitialized/default calibration values (NaN for pedestals, -1 for sampmax)

Potential Risk Areas

  • Reconstruction quality: Using default/uncalibrated values could degrade physics reconstruction quality if fallback calculation is not robust
  • Diagnostic visibility: Runs without proper calibrations may succeed without explicit failure, requiring careful monitoring of log output to identify calibration issues
  • Data consistency: Different data processing chains may produce different results depending on calibration availability
  • Performance: If on-the-fly calibration calculation is expensive, performance could be impacted for runs without pre-computed calibrations

Recommendations for Future Work

  • Implement robust on-the-fly calibration calculation routines to properly replace missing pre-computed calibrations
  • Add verbose logging and quality metrics to track when calibrations are missing vs. used
  • Consider adding configuration flags to enforce strict calibration requirements for critical physics analyses

Note: AI analysis tools can make mistakes when analyzing diffs. Reviewers should verify the actual impact on calibration logic by examining the complete Download_Ped() and Download_SampMax() implementations and confirming that missing calibrations will not cause downstream crashes or silent physics errors.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 1, 2026

📝 Walkthrough

Walkthrough

Removes runtime validation checks in MbdCalib.cc that previously returned error status when required calibration URLs (sampmax_url, ped_url) were absent. The code now continues execution instead of early-failing when these inputs are missing, deferring error handling to downstream operations.

Changes

Cohort / File(s) Summary
MBD Calibration Error Handling
offline/packages/mbd/MbdCalib.cc
Removed early-return guards from Download_All, Download_Ped, and Download_SampMax that previously set _status to -1 and aborted when required calibration sources were unavailable. Code now proceeds with missing inputs instead of failing fast.

Possibly related PRs

✨ Finishing touches
  • 📝 Generate docstrings

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.

@pinkenburg
Copy link
Contributor

merging this now to run this in online production

@pinkenburg pinkenburg merged commit d780913 into sPHENIX-Collaboration:master Feb 1, 2026
5 of 9 checks passed
@sphenix-jenkins-ci
Copy link

Build & test report

Report for commit 278cf7399958ccde29102c461c49a212b972a31b:
Jenkins on fire


Automatically generated by sPHENIX Jenkins continuous integration
sPHENIX             jenkins.io

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