Skip to content

Diffuse Laser Workflow#1351

Open
bkimelman wants to merge 6 commits into
sPHENIX-Collaboration:masterfrom
bkimelman:master
Open

Diffuse Laser Workflow#1351
bkimelman wants to merge 6 commits into
sPHENIX-Collaboration:masterfrom
bkimelman:master

Conversation

@bkimelman

@bkimelman bkimelman commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Added macros to run diffuse laser workflow

AI can make mistakes, so please use best judgment when reviewing.

Motivation / context: add macros for the diffuse laser workflow, align terminology from “TPC lamination correction” to “TPC average correction,” and remove reliance on private laser-matching QA code.

Key changes:

  • Added new Fun4All macros for:
    • laser clustering
    • laser matching
    • lamination fitting
  • These macros set up CDB/run configuration, input DST handling, tracking initialization, and workflow-specific output files/nodes.
  • Updated reconstruction macros to reference ENABLE_AVERAGE_CORRECTIONS / average corrections terminology.
  • Removed/avoided the private laser matching QA module call from the workflow.
  • Minor whitespace-only edits in existing tracking macros.

Potential risk areas:

  • New macro IO/output naming and DST/PDF file generation
  • Reconstruction behavior differences from the new laser workflow configuration
  • Correction-flag changes affecting tracking/TPC calibration results
  • Workflow runtime/performance impact from additional laser processing steps

Possible future improvements:

  • Add validation/QA checks for the new macros against reference data
  • Document expected input file conventions and output products
  • Consider consolidating shared configuration between the new laser macros

@coderabbitai

coderabbitai Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 76c5c861-118e-47ec-b491-da7b5020bd14

📥 Commits

Reviewing files that changed from the base of the PR and between edf9101 and 06a116e.

📒 Files selected for processing (6)
  • TrackingProduction/Fun4All_FullReconstruction.C
  • TrackingProduction/Fun4All_LaminationFitting.C
  • TrackingProduction/Fun4All_LaserClustering.C
  • TrackingProduction/Fun4All_LaserMatching.C
  • TrackingProduction/Fun4All_TrackAnalyzer.C
  • TrackingProduction/Fun4All_TrackSeeding.C
💤 Files with no reviewable changes (3)
  • TrackingProduction/Fun4All_TrackSeeding.C
  • TrackingProduction/Fun4All_FullReconstruction.C
  • TrackingProduction/Fun4All_TrackAnalyzer.C

📝 Walkthrough

Walkthrough

Three new Fun4All macros are added to TrackingProduction: Fun4All_LaserClustering for TPC laser hit unpacking and clustering, Fun4All_LaserMatching for central membrane matching, and Fun4All_LaminationFitting for lamination fits. Three existing macros receive whitespace-only fixes (one blank line removed each).

Changes

TPC Laser and Lamination Macros

Layer / File(s) Summary
Fun4All_LaserClustering macro
TrackingProduction/Fun4All_LaserClustering.C
New macro bootstrapping TPC laser hit unpacking (iterating 24 ebdc channels with optional endpoint splitting), laser event identification, laser clustering, and DST output with LaserEventInfo and LASER_CLUSTER nodes.
Fun4All_LaserMatching macro
TrackingProduction/Fun4All_LaserMatching.C
New macro initializing a TpcCentralMembraneMatching subsystem with conditional static/average distortion correction logic based on runnumber and an optional distortionCorrection argument; outputs per-run ROOT files under outdir.
Fun4All_LaminationFitting macro
TrackingProduction/Fun4All_LaminationFitting.C
New macro configuring TPC correction flags (including run-range-conditional static correction toggling), calling TPC_LaminationFitting(), and writing ROOT/PDF output filenames derived from outdir, type, prod, and runnumber.
Whitespace fixes in existing macros
TrackingProduction/Fun4All_FullReconstruction.C, TrackingProduction/Fun4All_TrackAnalyzer.C, TrackingProduction/Fun4All_TrackSeeding.C
One blank line removed immediately after G4TPC::ENABLE_AVERAGE_CORRECTIONS = true; in each file; no logic changes.

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.

@sphenix-jenkins-ci

Copy link
Copy Markdown

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 06a116e23a8ef87b87d0544041a5cec3f9f4c150. 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

@pinkenburg

Copy link
Copy Markdown
Contributor

The clang tidy errors from the streaming bco macros are not from this PR (they need a merge of the accompanying coresoftware PR), but the first three are. Please order includes properly:
First (followed by a blank line, so clang-format does not reorder this) GlobalVariables.C
then the macros from common (all *.C)
then the includes from our coresoftware.
stdio.h is legacy and should be removed. Some C++ includes are missing (e.g. fstream) to resolve some clang-diagnostic errors (root includes those automatically, which is just bad practice - clang-tidy does not)

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