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

mkFit updates aiming to improve pixelLess iteration and related #39055

Merged
merged 14 commits into from Aug 17, 2022

Conversation

slava77
Copy link
Contributor

@slava77 slava77 commented Aug 14, 2022

Motivated by observed inefficiency in the pixelLess iteration track building for low-pt displaced tracks, several features/bugs were fixed and improvements to the algorithm were added:

  • consistently sort seeds in iterations without seed merging
  • fix handling of candidates with overlapping hits
  • make sure that Kalman updates in the barrel stay on the update plane
  • fix a propagation condition check bug in propagation to radius calculation of several Jacobian elements
  • fix functions to count the number of matched hits and layers (used in pixelLess and detachedTriplet trajectory filtering)
  • explicitly account for the layer thickness in the endcap to determine the phi window to preselect hits (active only for the pixelLess iteration now)
  • relax the number of hit requirement for displaced track candidate filter in the pixelLess (endcaps only)
  • disable track candidate DNN selection in the pixelLess iteration
  • modify seed merging logic for the lowPtQuad iteration for the future developments (this iteration is not yet in the mkFit defaul production setup)

MTV plots, including timing, are available for ttbar relval with pileup [for the standard DQM selections](https://slava77.web.cern.ch/slava77/mic/CMSSW_12_5_0_pre4-pixmkf /RelValTTbar_14TeV/mtv-mkf5-mkf6-orig-Aug15POG-tr102-08be050) (signal tracks have pt>0.9 GeV in the efficiency plots) and separately [for signal simtracks selected with pt<0.9 GeV](https://slava77.web.cern.ch/slava77/mic/CMSSW_12_5_0_pre4-pixmkf/RelValTTbar_14TeV/mtv-mkf5-mkf6-orig-Aug15POG-tr102-08be050-lopt for low-pt)

Expectations:

  • slightly longer tracks in all mkFit iterations
  • a significant increase in efficiency of reconstructing displaced tracks in the pixelLess iteration
  • tracking time goes up by about 3% mainly from longer track fitting time downstream of mkFit, a large fraction of which is from disabling of the candidate DNN selection in the pixelLess iteration

Incremental comparisons are available in trackreco#102

Presentation in the Aug 15 TRK POG meeting https://indico.cern.ch/event/1189133/#3-mkfit-updates

Please consider for 12_5_0_pre5

@slava77
Copy link
Contributor Author

slava77 commented Aug 14, 2022

type tracking

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-39055/31557

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @slava77 (Slava Krutelyov) for master.

It involves the following packages:

  • RecoTracker/IterativeTracking (reconstruction)
  • RecoTracker/MkFitCMS (reconstruction)
  • RecoTracker/MkFitCore (reconstruction)

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

cms-bot commands are listed here

@slava77
Copy link
Contributor Author

slava77 commented Aug 14, 2022

@cmsbuild please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-edbc39/26807/summary.html
COMMIT: 08be050
CMSSW: CMSSW_12_5_X_2022-08-14-0000/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/39055/26807/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 20473 differences found in the comparisons
  • DQMHistoTests: Total files compared: 51
  • DQMHistoTests: Total histograms compared: 3692476
  • DQMHistoTests: Total failures: 33628
  • DQMHistoTests: Total nulls: 4
  • DQMHistoTests: Total successes: 3658822
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: -0.012 KiB( 50 files compared)
  • DQMHistoSizes: changed ( 138.4 ): -0.004 KiB JetMET/SUSYDQM
  • DQMHistoSizes: changed ( 138.5 ): -0.008 KiB JetMET/SUSYDQM
  • Checked 212 log files, 49 edm output root files, 51 DQM output files
  • TriggerResults: found differences in 3 / 50 workflows

@jpata
Copy link
Contributor

jpata commented Aug 16, 2022

@cms-sw/tracking-pog-l2 were there any outstanding questions you would like to be addressed from the meeting yesterday? (I did not have that impression, but just to check)

@mmusich
Copy link
Contributor

mmusich commented Aug 16, 2022

were there any outstanding questions you would like to be addressed from the meeting yesterday?

I don't think there was any specific objection to let this PR be integrated, provided it wasn't reactivating mkFit on the pixelLess iteration by default (that would require more work). Having said that, I haven't yet looked at the code changes if that's a review that is requested.

@jpata
Copy link
Contributor

jpata commented Aug 17, 2022

Thanks. It was more of a generic question than a request for review (which is of course always welcome).

@jpata
Copy link
Contributor

jpata commented Aug 17, 2022

+reconstruction

  • bugfixes for mkFit
  • consequently, changes in tracks and downstream quantities (see the slides in the description for details)
  • a slowdown of ~3% in tracking is expected from this

@cmsbuild
Copy link
Contributor

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 now be reviewed by the release team before it's merged. @perrotta, @dpiparo, @qliphy, @rappoccio (and backports should be raised in the release meeting by the corresponding L2)

@perrotta
Copy link
Contributor

+1

  • Modifications in tracking related quantities have been approved

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.

None yet

8 participants