Skip to content

fix pileup handling for oo in CreateFileList.pl#4133

Merged
pinkenburg merged 1 commit intosPHENIX-Collaboration:masterfrom
pinkenburg:oo-pileup
Jan 22, 2026
Merged

fix pileup handling for oo in CreateFileList.pl#4133
pinkenburg merged 1 commit intosPHENIX-Collaboration:masterfrom
pinkenburg:oo-pileup

Conversation

@pinkenburg
Copy link
Copy Markdown
Contributor

@pinkenburg pinkenburg commented Jan 22, 2026

Types of changes

  • [ X] 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, ...)

The handling of the pileup selection was wrong for OO (and AuAu for selected pileup scenarios). no jenkins running: [skip-ci]

TODOs (if applicable)

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

Fix Pileup Handling for OO in CreateFileList.pl

Motivation / Context

The CreateFileList.pl script generates dataset file lists for sPHENIX based on production type and collision system. The O+O (OO) collision system (production type 37) was incorrectly using Au+Au pileup background configuration, despite having different nuclear geometry. O+O collisions have a 0-15fm impact parameter range, whereas Au+Au uses 0-10fm. This PR corrects the pileup string generation for OO datasets to use system-specific background parameters.

Key Changes

  • Added OO-specific variables: Introduced $OO_pileupstring variable and $OO_bkgpileup constant set to _bkg_0_15fm (matching the 0-15fm impact parameter range for O+O collisions)
  • Extended pileup initialization logic: Modified the pileup rate calculation section (lines 174-201) to compute $OO_pileupstring for custom pileup rates (pileup > 5), combining the OO-specific background with the requested pileup rate
  • Clear pileup when background-less embedding is requested: Added line 206 to clear $OO_pileupstring alongside other system pileup strings when the --nobkgpileup flag is set (used for embedding without background pileup)
  • Fixed OO filename generation: Changed line 943 to use $OO_pileupstring instead of $AuAu_pileupstring when constructing sHijing_OO dataset filenames, ensuring O+O samples have correct pileup-dependent naming
  • Preserved nopileup behavior: Maintained existing logic to generate filenames without pileup information when the --nopileup flag is used

Potential Risk Areas

  • File naming convention change: O+O collision datasets will now use _<rate>kHz_bkg_0_15fm naming instead of the incorrect _<rate>kHz_bkg_0_10fm. This affects file discovery patterns and file catalog entries. Users and analysis scripts that hardcode filename patterns for O+O samples need to be updated.
  • Dataset metadata updates: File catalog and dataset registry entries for all O+O samples across different pileup configurations may require synchronization.
  • Downstream logic dependency: Line 946 sets $pileupstring = $AuAu_pileupstring; for OO production type. This assignment (which should arguably use $OO_pileupstring for consistency) may affect downstream file type assignment logic and was not corrected in this PR.

Possible Future Improvements

  • Consider updating line 946 to use $OO_pileupstring to maintain consistent pileup string tracking throughout the script
  • Refactor the pileup initialization logic (lines 174-201) to reduce code duplication across collision systems using helper functions or data structures
  • Add validation or unit tests to verify correct filename generation for each collision system and pileup configuration combination

Note: AI-assisted analysis of complex Perl scripts can contain interpretation errors. Review of file naming impacts and verification that downstream workflows properly handle the new O+O naming convention is strongly recommended.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Jan 22, 2026

📝 Walkthrough

Walkthrough

This PR introduces OO-specific pileup configuration variables (OO_pileupstring and OO_bkgpileup) to the file list creation framework, enabling proper pileup string construction for oxygen-oxygen collision scenarios (prodtype 37) in the Hijing generator, replacing the previous Au+Au pileup string usage.

Changes

Cohort / File(s) Summary
OO pileup configuration
offline/framework/frog/CreateFileList.pl
Added OO_pileupstring and OO_bkgpileup variables for O+O-specific pileup handling. Extended initialization logic to compute OO_pileupstring by combining OO background with pileup rate. Updated prodtype 37 (Hijing O+O) file naming to use OO_pileupstring instead of AuAu_pileupstring. Properly clears pileup strings in no-background-pileup scenarios.

Possibly related PRs

  • add type 37: hijing O+O 0-15fm #4107: Preceding PR that modified prodtype 37 handling in the same file; this PR extends that work by adding O+O-specific pileup variables and switching filename construction to use OO_pileupstring.

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.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Caution

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

⚠️ Outside diff range comments (2)
offline/framework/frog/CreateFileList.pl (2)

173-201: Fix Perl syntax error and initialize OO pileup for standard modes.

Line 200 has a hard syntax error ($ OO_bkgpileup) that prevents the script from running. Also, OO_pileupstring is only set in the else branch; for pileup modes 1–3 (the defaults) it stays undefined, so OO filenames silently miss pileup suffixes.

🔧 Proposed fix
 if ($pileup == 1)
 {
     $AuAu_pileupstring = sprintf("_50kHz%s",$AuAu_bkgpileup);
     $pp_pileupstring = sprintf("_3MHz");
     $pAu_pileupstring = sprintf("_500kHz%s",$pAu_bkgpileup);
+    $OO_pileupstring = sprintf("_50kHz%s",$OO_bkgpileup);
 }
 elsif ($pileup == 2)
 {
     $AuAu_pileupstring = sprintf("_25kHz%s",$AuAu_bkgpileup);
+    $OO_pileupstring = sprintf("_25kHz%s",$OO_bkgpileup);
 }
 elsif ($pileup == 3)
 {
     $AuAu_pileupstring = sprintf("_10kHz%s",$AuAu_bkgpileup);
+    $OO_pileupstring = sprintf("_10kHz%s",$OO_bkgpileup);
 }
 ...
 else
 {
     $pp_pileupstring = sprintf("_%dkHz",$pileup);
     $AuAu_pileupstring = sprintf("_%dkHz%s",$pileup, $AuAu_bkgpileup);
-    $OO_pileupstring = sprintf("_%dkHz%s",$pileup,$ OO_bkgpileup);
+    $OO_pileupstring = sprintf("_%dkHz%s",$pileup, $OO_bkgpileup);
 }

943-947: Use the OO pileup string for OO production type.

For prodtype == 37, $pileupstring is still set to $AuAu_pileupstring. This makes later filename manipulations (e.g., G4Hits path rewriting) use the wrong suffix for OO.

🔧 Proposed fix
-        $pileupstring = $AuAu_pileupstring;
+        $pileupstring = $OO_pileupstring;

@pinkenburg pinkenburg merged commit 747eba0 into sPHENIX-Collaboration:master Jan 22, 2026
2 checks passed
@coderabbitai coderabbitai bot mentioned this pull request Jan 22, 2026
4 tasks
@coderabbitai coderabbitai bot mentioned this pull request Feb 16, 2026
4 tasks
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