Skip to content

Fix: DetermineTowerBackground - Add Psi2 Safety Check#4136

Merged
pinkenburg merged 1 commit intosPHENIX-Collaboration:masterfrom
Steepspace:jetbackground-fix
Jan 24, 2026
Merged

Fix: DetermineTowerBackground - Add Psi2 Safety Check#4136
pinkenburg merged 1 commit intosPHENIX-Collaboration:masterfrom
Steepspace:jetbackground-fix

Conversation

@Steepspace
Copy link
Copy Markdown
Contributor

@Steepspace Steepspace commented Jan 22, 2026

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, ...)

Bug Fix: Ensure Psi2 from EventplaneinfoMap is finite and not NaN which can corrupt downstream calculations.

TODOs (if applicable)

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

Pull Request Summary: DetermineTowerBackground - Add Psi2 Safety Check

Motivation & Context

The sEPD (sPHENIX Event Plane Detector) is used to extract the event plane angle (Psi2) for flow-corrected analyses in the tower background determination module. In rare cases, the EventplaneinfoMap data retrieval can return non-finite values (NaN or Inf), which would corrupt downstream calculations of event plane and anisotropic flow coefficients. This fix adds a defensive safety check to catch and handle such corrupted values.

Key Changes

  • Added finite-value check for _Psi2 immediately after extraction from sEPD data (line ~615)
    • Validates using std::isfinite(_Psi2) to detect both NaN and Inf values
    • Complements existing similar checks in the calo event plane branch
  • Fallback behavior on detection:
    • Sets flow failure flag (_is_flow_failure = true)
    • Resets _Psi2 to 0 to prevent propagation of corrupt values
    • Emits diagnostic warning message when verbosity enabled
  • Minimal, targeted change: +11 lines of defensive code only

Potential Risk Areas

  • Data quality monitoring: Events flagged with _is_flow_failure will have _Psi2 = 0, which may merge with legitimate zero-flow events; monitoring rates useful for detecting persistent sEPD issues
  • No reconstruction behavior changes: This is purely a guard against corrupted input data; valid flows are unaffected
  • No performance impact: Simple floating-point check with negligible overhead

Future Improvements

  • Consider logging event statistics (run/event IDs) of failures for sEPD diagnostic purposes
  • Monitor correlation between _is_flow_failure and detector data quality flags
  • Evaluate if similar safety checks needed for other event plane detectors

Note: This analysis relies on AI-generated interpretation of code structure; careful review of EventplaneinfoMap behavior and sEPD data sources recommended to ensure check addresses root causes rather than symptoms.

- Ensure Psi2 from EventplaneinfoMap is finite and not NaN which can corrupt downstream calculations.
Copilot AI review requested due to automatic review settings January 22, 2026 22:24
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Jan 22, 2026

📝 Walkthrough

Walkthrough

Adds a safety check to ensure _Psi2 is finite in the sEPD flow branch. When non-finite values (NaN/Inf) are detected, the code logs a warning (if verbosity > 0), sets a flow failure flag, and resets _Psi2 to zero, augmenting existing non-finite value handling.

Changes

Cohort / File(s) Summary
sEPD Flow Validation
offline/packages/jetbackground/DetermineTowerBackground.cc
Adds finite-value validation for _Psi2 post-extraction from sEPD flow; logs warning and sets flow failure flag when non-finite values detected
✨ 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.

@sphenix-jenkins-ci
Copy link
Copy Markdown

Build & test report

Report for commit 3fa48da17176b340a61716e6a0ab2aa69ea4894d:
Jenkins on fire

Build & test report

Report for commit 9ec298fb7d3b94a243e8056445b7879d11f58659:
Jenkins on fire

Build & test report

Report for commit 18c5a2c56900618bcf9798bb091f064b628994a4:
Jenkins on fire

Build & test report

Report for commit f704495167ff5a179e3b400f49850351b7084714:
Jenkins passed


Automatically generated by sPHENIX Jenkins continuous integration
sPHENIX             jenkins.io


Automatically generated by sPHENIX Jenkins continuous integration
sPHENIX             jenkins.io


Automatically generated by sPHENIX Jenkins continuous integration
sPHENIX             jenkins.io


Automatically generated by sPHENIX Jenkins continuous integration
sPHENIX             jenkins.io

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds a safety check to prevent non-finite (NaN or Inf) values in the Psi2 event plane angle when extracted from the EventplaneinfoMap during sEPD event plane reconstruction. The check prevents corrupted Psi2 values from propagating to downstream flow calculations.

Changes:

  • Added validation check for non-finite Psi2 values after extraction from EventplaneinfoMap
  • Sets Psi2 to 0 and marks flow extraction as failed when non-finite values are detected
  • Adds warning message when non-finite values are encountered

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@pinkenburg pinkenburg merged commit f810048 into sPHENIX-Collaboration:master Jan 24, 2026
7 of 10 checks passed
@Steepspace Steepspace deleted the jetbackground-fix branch January 24, 2026 02:13
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.

3 participants