Skip to content

Fixed library dependency (libtrack_reco -> libtrackbase_historic).#4140

Merged
osbornjd merged 2 commits intosPHENIX-Collaboration:masterfrom
hupereir:fix_dedx_dependencies
Jan 28, 2026
Merged

Fixed library dependency (libtrack_reco -> libtrackbase_historic).#4140
osbornjd merged 2 commits intosPHENIX-Collaboration:masterfrom
hupereir:fix_dedx_dependencies

Conversation

@hupereir
Copy link
Contributor

@hupereir hupereir commented Jan 26, 2026

This allows to remove openmp configuration flag, introduced in #4139

See discussion at #4139

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

TODOs (if applicable)

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

Motivation and Context

This PR resolves library dependency issues in the dEdx fitter calibration module and removes an unnecessary OpenMP compilation flag. The changes allow cleanup of build configuration introduced in PR #4139, which added the -fopenmp flag due to perceived OpenMP dependencies that do not actually exist in the code.

Key Changes

  • Makefile.am: Replaced -ltrack_reco with -ltrackbase_historic in the libdedxfitter_la_LIBADD linkage. The module already includes headers from trackbase_historic (specifically SvtxTrackMap.h and TrackAnalysisUtils.h) and was already linking against libtrackbase_historic and libtrackbase_historic_io.
  • configure.ac: Removed the -fopenmp compilation flag from CXXFLAGS, eliminating an unnecessary OpenMP dependency that was not actually used in the code.

Risk Assessment

  • Low Risk: The code contains no OpenMP pragmas (#pragma omp) or OpenMP API calls, confirming the flag was unnecessary.
  • No Behavioral Impact: The dEdx fitting algorithms (contained in GlobaldEdxFitter and dEdxFitter) rely on ROOT mathematical minimization and track analysis utilities from trackbase_historic, not on parallelization.
  • Transitive Dependencies: The libtrack_reco library dependency appears to have been redundant, as the required functionality is already available through the direct libtrackbase_historic dependency.
  • Build Simplification: Removes outdated/incorrect build configuration without affecting runtime behavior.

Possible Future Improvements

  • Review and update the comment in configure.ac that claims "this package needs openmp," which appears to be outdated documentation.
  • Consider verifying across the codebase whether other modules with similar comment patterns have the same unnecessary OpenMP dependencies.

Note: AI-assisted analysis has been used in identifying these patterns. Manual verification of transitive dependencies and actual library requirements is recommended.

This allows to remove openmp configuration flag, introduced in sPHENIX-Collaboration#4139
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 26, 2026

📝 Walkthrough

Walkthrough

The changes update build configuration for the TPC dEdx calibration module by modifying library dependencies in the Makefile and removing the OpenMP compilation flag from the configure script.

Changes

Cohort / File(s) Summary
Build Configuration
calibrations/tpc/dEdx/Makefile.am, calibrations/tpc/dEdx/configure.ac
Updated library linkage by replacing -ltrack_reco with -ltrackbase_historic in libdedxfitter\_la\_LIBADD; removed -fopenmp flag from CXXFLAGS while preserving other compiler flags (-Wall, -Wshadow, -Wextra, -Werror)

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

Build & test report

Report for commit 5e38c107eb93abe133ef477bc60ab25b3848b1a6:
Jenkins passed


Automatically generated by sPHENIX Jenkins continuous integration
sPHENIX             jenkins.io

@osbornjd osbornjd merged commit 349b8f6 into sPHENIX-Collaboration:master Jan 28, 2026
22 checks passed
@hupereir hupereir deleted the fix_dedx_dependencies branch February 10, 2026 17:56
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