Skip to content

Adds local PID parametrization file option to KFParticle_sPHENIX#4137

Merged
osbornjd merged 1 commit intosPHENIX-Collaboration:masterfrom
petermic:kfparticle_localpidfile
Feb 2, 2026
Merged

Adds local PID parametrization file option to KFParticle_sPHENIX#4137
osbornjd merged 1 commit intosPHENIX-Collaboration:masterfrom
petermic:kfparticle_localpidfile

Conversation

@petermic
Copy link
Contributor

@petermic petermic commented Jan 23, 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, ...)

Adds option to KFParticle_sPHENIX to use a local file for the dE/dx parametrization, rather than the file in the CDB. The default is still the CDB file. Enable in macro using useLocalPIDFile(true) and set filename using setLocalPIDFilename.

TODOs (if applicable)

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

Local PID Parametrization File Option for KFParticle_sPHENIX

Motivation

Adds the ability to use a local dE/dx parametrization file for particle identification (PID) instead of relying exclusively on the default CDB (Conditions Database) file. This feature allows greater flexibility for testing alternative parametrizations and local analysis workflows without modifying the database.

Key Changes

  • New configuration methods in KFParticle_sPHENIX:

    • useLocalPIDFile(bool use = false) — enables/disables local file mode (default: off)
    • setLocalPIDFilename(std::string name) — specifies the local file path
  • Modified init_dEdx_fits() function in KFParticle_Tools:

    • Conditionally loads parametrization from local file or CDB based on flag
    • When using local file: loads ROOT objects with charge-independent names (pi_band, K_band, p_band for all charge states)
    • When using CDB (default): loads objects with charge-specific names (f_piband, f_Kband, f_pband, f_piminus_band, f_Kminus_band, f_pbar_band)
    • Improved error handling with std::cerr output for file open failures
  • Lines changed: ~35 across three files (KFParticle_sPHENIX.h, KFParticle_Tools.h/cc)

Potential Risk Areas

  1. File Format Compatibility: Local PID files must contain ROOT TF1 objects with the specific naming convention (pi_band, K_band, p_band), which differs from the CDB format. Providing an incorrectly formatted file will fail silently after printing an error message.
  2. File I/O Dependencies: File not found or permission errors could break PID functionality mid-analysis.
  3. Backward Compatibility: Feature is off by default, so existing analyses are unaffected, but any new analyses using the feature must ensure the local file exists and is accessible.

Possible Future Improvements

  • Add validation/test of file format before use (e.g., check for required object names)
  • Implement fallback mechanism (attempt CDB if local file fails)
  • Add logging/informational messages about which source is being used
  • Consider supporting additional parametrization formats or file types

Note: AI-generated summaries can contain inaccuracies. The core logic appears sound (conditional file loading based on a boolean flag), but reviewers should verify that the expected file format and object naming scheme match actual usage patterns in local PID files.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 23, 2026

📝 Walkthrough

Walkthrough

Addition of local PID file configuration support to KFParticle framework. Introduces public methods useLocalPIDFile() and setLocalPIDFilename() to configure this feature, along with corresponding member variables. Modifies dEdx fit parameter loading in init_dEdx_fits() to conditionally source parameters and band objects based on whether local file mode is enabled.

Changes

Cohort / File(s) Summary
Public Interface
KFParticle_sPHENIX.h
Added useLocalPIDFile() and setLocalPIDFilename() methods for configuring local PID file usage
Backend Configuration
KFParticle_Tools.h, KFParticle_Tools.cc
Added member variables m_use_local_PID_file and m_local_PID_filename; refactored init_dEdx_fits() to conditionally load fit parameters from local file or CDB based on configuration flag; improved error handling with std::cerr
✨ 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.

Copy link
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/packages/KFParticle_sPHENIX/KFParticle_Tools.cc (2)

1205-1232: Validate TF1 object lookups before populating pidMap.

GetObject() returns without modifying the pointer if the object is missing, leaving it nullptr. These null pointers then flow into pidMap and will crash get_dEdx_fitValue() when it dereferences pidMap[PID]->Eval(momentum). Add validation checks and return early if any required TF1 object is not found.

Suggested fix
   if (m_use_local_PID_file)
   {
     // new method is independent of charge
     filefit->GetObject("pi_band",f_pion_plus);
     filefit->GetObject("K_band",f_kaon_plus);
     filefit->GetObject("p_band",f_proton_plus);
     filefit->GetObject("pi_band",f_pion_minus);
     filefit->GetObject("K_band",f_kaon_minus);
     filefit->GetObject("p_band",f_proton_minus);
   }
   else
   {
     filefit->GetObject("f_piband", f_pion_plus);
     filefit->GetObject("f_Kband", f_kaon_plus);
     filefit->GetObject("f_pband", f_proton_plus);
     filefit->GetObject("f_piminus_band", f_pion_minus);
     filefit->GetObject("f_Kminus_band", f_kaon_minus);
     filefit->GetObject("f_pbar_band", f_proton_minus);
   }
+
+  const bool missing =
+      !f_pion_plus || !f_kaon_plus || !f_proton_plus ||
+      !f_pion_minus || !f_kaon_minus || !f_proton_minus;
+  if (missing)
+  {
+    std::cerr << "Missing PID TF1(s) in file: " << dedx_fitparams << std::endl;
+    return;
+  }

1187-1203: Guard against null TFile::Open results before dereference.

TFile::Open returns nullptr on failure (missing file, invalid path, permissions). The code dereferences filefit at line 1199 with filefit->IsOpen() without checking for null first—if m_local_PID_filename is empty or invalid, this will crash.

Add a null check before dereferencing:

Proposed fix
   TFile *filefit = TFile::Open(dedx_fitparams.c_str());
-
-  if (!filefit->IsOpen())
+  if (!filefit || !filefit->IsOpen())
   {
-    std::cerr << "Error opening filefit!" << std::endl;
+    std::cerr << "Error opening dEdx fit file: " << dedx_fitparams << std::endl;
     return;
   }

@cdean-github
Copy link
Contributor

Hi @petermic, thanks for adding this is. I think it does what we want but I'd suggest one change:

useLocalPIDFile(bool use = false)

this default here should probably be true. I see that m_use_local_PID_file is false by default but the value in the function above will also set it to false if no arguement is given so a user doing kfparticle->useLocalPIDFile() will use the CDB PID file.

@osbornjd osbornjd merged commit 79a1639 into sPHENIX-Collaboration:master Feb 2, 2026
2 of 4 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.

3 participants