Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce procModifier for parabolic magnetic field in track building for default iterative tracking #41557

Merged
merged 3 commits into from
May 9, 2023

Conversation

mmasciov
Copy link
Contributor

@mmasciov mmasciov commented May 5, 2023

PR description:

As per title, this PR is introducing a process modifier to enable the usage of parabolic (rather than full) magnetic field for track building, for the default iterative tracking.
It follows a presentation at the TRK POG.

The process modifier is NOT enabled by default.
To enable process modifier, the following addition to cmsDriver.py command for step3 is sufficient:
--procModifiers=trackingParabolicMf

For documentation: the usage of the parabolic magnetic field in track building was (unintentionally) reverted in CMSSW 71X, as detailed in issue #41396.

In addition, commit eaf188f:

PR validation:

As is, this is a technical PR: by default, the process modifier is NOT enabled.
To summarize, when the process modifier is enabled:

  • For tracking physics performance: impact on efficiency and fake rate is negligible; impact on resolution is negligible; impact on pT residual at low pT is visible, although small.
  • For tracking timing performance: overall track building (full tracking) time is reduced by less than 5% (~2%).

More details can be found in TRK POG presentation, or in the linked MTV results for TTbar events with PU.

FYI, @mmusich, @slava77

@cmsbuild
Copy link
Contributor

cmsbuild commented May 5, 2023

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-41557/35431

  • This PR adds an extra 60KB to repository

  • There are other open Pull requests which might conflict with changes you have proposed:

@cmsbuild
Copy link
Contributor

cmsbuild commented May 5, 2023

A new Pull Request was created by @mmasciov (Mario Masciovecchio) for master.

It involves the following packages:

  • Configuration/ProcessModifiers (operations)
  • RecoTracker/CkfPattern (reconstruction)
  • RecoTracker/IterativeTracking (reconstruction)
  • RecoTracker/TkNavigation (reconstruction)

@perrotta, @rappoccio, @clacaputo, @cmsbuild, @mandrenguyen, @fabiocos, @davidlange6 can you please review it and eventually sign? Thanks.
@fabiocos, @VourMa, @makortel, @felicepantaleo, @GiacomoSguazzoni, @JanFSchulte, @rovere, @VinInn, @Martin-Grunewald, @missirol, @ebrondol, @gpetruc, @mmusich, @mtosi, @dgulhan this is something you requested to watch as well.
@perrotta, @dpiparo, @rappoccio you are the release manager for this.

cms-bot commands are listed here

@cmsbuild
Copy link
Contributor

cmsbuild commented May 5, 2023

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-41557/35435

  • This PR adds an extra 52KB to repository

  • There are other open Pull requests which might conflict with changes you have proposed:

Code check has found code style and quality issues which could be resolved by applying following patch(s)

@slava77
Copy link
Contributor

slava77 commented May 5, 2023

I don't see changes in DisplacedGeneralStep_cff.py and DisplacedRegionalStep_cff.py.
I guess we missed them for fitting as well

@mmasciov
Copy link
Contributor Author

mmasciov commented May 5, 2023

I don't see changes in DisplacedGeneralStep_cff.py and DisplacedRegionalStep_cff.py. I guess we missed them for fitting as well

Yes, no change in those so far. Can be propagated if wished; however, those configurations are not run by default, and all these and previous changes only targeted default iterations. Thus, will not do unless requested.

@cmsbuild
Copy link
Contributor

cmsbuild commented May 5, 2023

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-41557/35436

  • This PR adds an extra 52KB to repository

  • There are other open Pull requests which might conflict with changes you have proposed:

@cmsbuild
Copy link
Contributor

cmsbuild commented May 5, 2023

Pull request #41557 was updated. @perrotta, @rappoccio, @clacaputo, @cmsbuild, @mandrenguyen, @fabiocos, @davidlange6 can you please check and sign again.

@mmasciov
Copy link
Contributor Author

mmasciov commented May 8, 2023

I don't see changes in DisplacedGeneralStep_cff.py and DisplacedRegionalStep_cff.py. I guess we missed them for fitting as well

Yes, no change in those so far. Can be propagated if wished; however, those configurations are not run by default, and all these and previous changes only targeted default iterations. Thus, will not do unless requested.

As a change was necessary in DisplacedRegionalStep_cff.py, I ended up propagating changes to both.

@cmsbuild
Copy link
Contributor

cmsbuild commented May 8, 2023

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-41557/35467

  • This PR adds an extra 60KB to repository

  • There are other open Pull requests which might conflict with changes you have proposed:

@cmsbuild
Copy link
Contributor

cmsbuild commented May 8, 2023

Pull request #41557 was updated. @perrotta, @rappoccio, @clacaputo, @cmsbuild, @mandrenguyen, @fabiocos, @davidlange6 can you please check and sign again.

@slava77
Copy link
Contributor

slava77 commented May 8, 2023

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented May 8, 2023

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-0227aa/32474/summary.html
COMMIT: 1bc9f4d
CMSSW: CMSSW_13_2_X_2023-05-08-1100/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/41557/32474/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • You potentially added 9 lines to the logs
  • Reco comparison results: 4 differences found in the comparisons
  • DQMHistoTests: Total files compared: 48
  • DQMHistoTests: Total histograms compared: 3460836
  • DQMHistoTests: Total failures: 3
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3460811
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
  • Checked 207 log files, 159 edm output root files, 48 DQM output files
  • TriggerResults: no differences found

@clacaputo
Copy link
Contributor

+reconstruction

@rappoccio
Copy link
Contributor

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented May 9, 2023

This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will be automatically merged.

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.

6 participants