Skip to content

CM Distortion variables and Macro Updates#1297

Merged
osbornjd merged 4 commits intosPHENIX-Collaboration:masterfrom
bkimelman:master
Feb 24, 2026
Merged

CM Distortion variables and Macro Updates#1297
osbornjd merged 4 commits intosPHENIX-Collaboration:masterfrom
bkimelman:master

Conversation

@bkimelman
Copy link
Contributor

@bkimelman bkimelman commented Feb 23, 2026

Addition of new flags and variables for use in setting CM Lamination Fitting options and calls to set macro using those new variables.

Uses PR sPHENIX-Collaboration/coresoftware#4184

CM Distortion Variables and Macro Updates

Motivation / Context

Expose Central Membrane (CM) lamination fitting controls to macros so users can configure distortion-correction behavior (stripe pattern, histogram outputs, and magnetic-field handling) without editing C++ sources. This enables easier testing, reproducibility, and macro-driven workflows for TPC lamination fitting.

Key changes

  • common/G4_TrkrVariables.C
    • Added G4TPC::CMStripePattern (std::string) = "/sphenix/u/bkimelman/CMStripePattern_ideal.root"
    • Added G4TPC::SaveAllLaminationHists (bool) = false
    • Added G4TPC::BFieldOff (bool) = false
    • Note: USE_PHI_AS_RAD_AVERAGE_CORRECTIONS remains enabled (true) — no change to its default in this PR.
  • common/Trkr_LaserClustering.C
    • Updated TPC_LaminationFitting initialization to use new variables:
      • set_phiHistInRad(G4TPC::USE_PHI_AS_RAD_AVERAGE_CORRECTIONS)
      • set_nLayerCut(1) (replaces previous set_nLayerCut(2))
      • set_fieldOff(G4TPC::BFieldOff)
      • set_stripePatternFile(G4TPC::CMStripePattern)
      • set_saveAllLaminationHistograms(G4TPC::SaveAllLaminationHists)

Potential risk areas

  • IO / config: Default CMStripePattern points to a user-specific path (/sphenix/u/bkimelman/...), which may be unavailable in other environments; missing or incorrect file can alter or break lamination fitting runs.
  • Reconstruction behavior: Using phi-as-radius-average corrections (flag already true) and lowering nLayerCut from 2→1 can change distortion-correction outcomes and QA — requires re-validation against physics baselines.
  • Operational safety: BFieldOff controls magnetic-field handling; accidental use may produce non-physical reconstructions.
  • Performance / thread-safety: Enabling SaveAllLaminationHists increases I/O and memory usage; no explicit thread-safety changes observed but verify lamination-fitting is safe in concurrent runs.

Possible future improvements

  • Replace hard-coded user path with a collaboration-wide config, relative path, or environment-variable override.
  • Add runtime validation for stripe-pattern file existence and clear error messages.
  • Document new flags (semantics, recommended defaults, and diagnostics) in macros and user guides.
  • Add warnings or safeguards when enabling BFieldOff in production-like workflows.
  • Consider finer-grained tuning (per-sector/module) and config versioning/checksum for reproducibility.

Note: AI-generated summaries can be mistaken or incomplete — review the actual diffs and run reconstruction/QA tests to confirm impacts before deploying.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 23, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5d45533 and 1a2235f.

📒 Files selected for processing (1)
  • common/G4_TrkrVariables.C

📝 Walkthrough

Walkthrough

Adds three new G4TPC configuration variables (CMStripePattern, SaveAllLaminationHists, BFieldOff) and wires them into TPC lamination fitting setup in common/Trkr_LaserClustering.C; adjusts phi-hist normalization and lowers the lamination layer cut.

Changes

Cohort / File(s) Summary
Configuration Variables
common/G4_TrkrVariables.C
Added std::string CMStripePattern = "/sphenix/u/bkimelman/CMStripePattern_ideal.root";, bool SaveAllLaminationHists = false;, and bool BFieldOff = false;. Existing USE_PHI_AS_RAD_AVERAGE_CORRECTIONS remains set (true).
Lamination Fitting Configuration
common/Trkr_LaserClustering.C
Replaced previous layer-cut/phi handling with set_phiHistInRad(G4TPC::USE_PHI_AS_RAD_AVERAGE_CORRECTIONS) and set_nLayerCut(1); added set_fieldOff(G4TPC::BFieldOff), set_stripePatternFile(G4TPC::CMStripePattern), and set_saveAllLaminationHistograms(G4TPC::SaveAllLaminationHists) when configuring TPC_LaminationFitting.

Possibly related PRs


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

For repository maintainers, please start the CI check manually (feedback)

This is an automatic message to assist manually starting CI check for this pull request, commit a2bfb83f43ba57e66303dd0b4c023448307e1006. macros pull request require a manual start for CI checks, in particular selecting which coresoftware and calibrations versions to check against this macros pull request.

sPHENIX software maintainers: please make your input here and start the Build:

build

Note:

  1. if needed, fill in the pull request ID for the coresoftware pull request, e.g. origin/pr/1697/merge for PR#1697 in sha_coresoftware. Default is to check with the master branch.
  2. click Build button at the end of the long web page to start the test

Automatically generated by sPHENIX Jenkins continuous integration
sPHENIX             jenkins.io

@sphenix-jenkins-ci
Copy link

For repository maintainers, please start the CI check manually (feedback)

This is an automatic message to assist manually starting CI check for this pull request, commit 5d45533f49eec507d4a299a538f6cdf337e8c081. macros pull request require a manual start for CI checks, in particular selecting which coresoftware and calibrations versions to check against this macros pull request.

sPHENIX software maintainers: please make your input here and start the Build:

build

Note:

  1. if needed, fill in the pull request ID for the coresoftware pull request, e.g. origin/pr/1697/merge for PR#1697 in sha_coresoftware. Default is to check with the master branch.
  2. click Build button at the end of the long web page to start the test

Automatically generated by sPHENIX Jenkins continuous integration
sPHENIX             jenkins.io

// average distortion corrections
bool ENABLE_AVERAGE_CORRECTIONS = false;
std::string average_correction_filename;
std::string CMStripePattern = "/sphenix/u/bkimelman/CMStripePattern_ideal.root";
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a file that should go in the CDB?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Eventually, yes. I still need to develop the finalized version of the stripe pattern based on field-off data. Until then, I think it's better to just use a local file

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay we can leave it as is for now, I think this will not cause a problem for anyone running on sdcc

@sphenix-jenkins-ci
Copy link

For repository maintainers, please start the CI check manually (feedback)

This is an automatic message to assist manually starting CI check for this pull request, commit 1a2235f1966c2e0f7ac012e553cf9da2cf8ee798. macros pull request require a manual start for CI checks, in particular selecting which coresoftware and calibrations versions to check against this macros pull request.

sPHENIX software maintainers: please make your input here and start the Build:

build

Note:

  1. if needed, fill in the pull request ID for the coresoftware pull request, e.g. origin/pr/1697/merge for PR#1697 in sha_coresoftware. Default is to check with the master branch.
  2. click Build button at the end of the long web page to start the test

Automatically generated by sPHENIX Jenkins continuous integration
sPHENIX             jenkins.io

@osbornjd osbornjd merged commit d5d77aa into sPHENIX-Collaboration:master Feb 24, 2026
3 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.

2 participants