Skip to content

Conversation

@tomalin
Copy link
Contributor

@tomalin tomalin commented Dec 6, 2025

PR description:

Some years ago, In the TTTrack class, which represents L1 tracks, the function stubPtConsistency() was renamed chi2BendRed(). The new name gave a clearer idea what this function did, which is to return a chi2/dof (also known as reduced chi2) that was formed from the stub bend information.
However, at that time a duplicate function with the original name was retained in TTTrack, to avoid breaking anyone's code.

This PR finally finally deletes function stubPtConsistency(), and updates the few classes that had not yet migrated to the new function name.

This has no effect on L1 tracking performance, since it is only a name change,

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 6, 2025

cms-bot internal usage

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 6, 2025

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-49565/47091

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 6, 2025

Pull request #49565 was updated.

@tomalin tomalin marked this pull request as ready for review December 6, 2025 20:08
@nothingface0
Copy link
Contributor

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 8, 2025

A new Pull Request was created by @tomalin for master.

It involves the following packages:

  • DQM/SiTrackerPhase2 (dqm)
  • DQMOffline/L1Trigger (dqm, l1)
  • DataFormats/L1TrackTrigger (l1)
  • L1Trigger/L1TTrackMatch (l1)
  • L1Trigger/TrackFindingTMTT (l1)
  • L1Trigger/TrackFindingTracklet (l1)
  • L1Trigger/TrackerTFP (l1)
  • L1Trigger/VertexFinder (l1)

@BenjaminRS, @ctarricone, @gabrielmscampos, @nothingface0, @quinnanm, @rseidita can you please review it and eventually sign? Thanks.
@Martin-Grunewald, @arossi83, @erikbutz, @missirol, @mmusich, @richa2710, @rociovilar, @rovere, @skinnari, @sroychow, @sviret this is something you requested to watch as well.
@ftenchini, @mandrenguyen, @sextonkennedy you are the release manager for this.

cms-bot commands are listed here

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 8, 2025

+1

Size: This PR adds an extra 16KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-2da15d/49839/summary.html
COMMIT: 5bfb8fa
CMSSW: CMSSW_16_0_X_2025-12-08-1100/el8_amd64_gcc13
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/49565/49839/install.sh to create a dev area with all the needed externals and cmssw changes.

DAS Queries: The DAS query tests failed, see the summary page for details.

Comparison Summary

Summary:

  • You potentially removed 1 lines from the logs
  • ROOTFileChecks: Some differences in event products or their sizes found
  • Reco comparison results: 4 differences found in the comparisons
  • Reco comparison had 4 failed jobs
  • DQMHistoTests: Total files compared: 53
  • DQMHistoTests: Total histograms compared: 4273241
  • DQMHistoTests: Total failures: 3
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 4273218
  • DQMHistoTests: Total skipped: 20
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 52 files compared)
  • Checked 227 log files, 198 edm output root files, 53 DQM output files
  • TriggerResults: no differences found

@nothingface0
Copy link
Contributor

+dqm

@cmsbuild
Copy link
Contributor

Milestone for this pull request has been moved to CMSSW_16_1_X. Please open a backport if it should also go in to CMSSW_16_0_X.

@cmsbuild cmsbuild modified the milestones: CMSSW_16_0_X, CMSSW_16_1_X Dec 18, 2025
@cmsbuild cmsbuild modified the milestones: CMSSW_16_1_X, CMSSW_16_0_X Dec 18, 2025
Comment on lines +736 to +738
float chi2BendRed =
StubPtConsistency::getConsistency(aTrack, theTrackerGeom, tTopo, settings_.bfield(), settings_.nHelixPar());
aTrack.setChi2BendRed(chi2BendRed);
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't believe we need to create a new variable chi2BendRed but instead can just use:

Suggested change
float chi2BendRed =
StubPtConsistency::getConsistency(aTrack, theTrackerGeom, tTopo, settings_.bfield(), settings_.nHelixPar());
aTrack.setChi2BendRed(chi2BendRed);
aTrack.setChi2BendRed(StubPtConsistency::getConsistency(aTrack, theTrackerGeom, tTopo, settings_.bfield(), settings_.nHelixPar()));

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True, but it's a matter of taste. I found two shorter lines more comprehensible than one long line. I can change it if you like.

return;
}

/// StubPtConsistency
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to have a templated method for the proper way the variable should be accessed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean, should function chi2BendRed() be templated? Why would it be? Which template argument do you want?
P.S. All data members of TTTrack are double, which seems a waste of space to me. Sometime we should replace them by float, but that would break backwards compatibility ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants