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

Fully implement fishbone in patatrack #36215

Merged
merged 22 commits into from
Dec 1, 2021
Merged

Conversation

VinInn
Copy link
Contributor

@VinInn VinInn commented Nov 23, 2021

I eventually fully implemented the fishbone algorithm.
The main differences w/r/t previous code are

when the fishbone identifies 2 collinear doublets between the same layers it chooses the longest and store the intermediate hit
Those "new" hits are added to the track (max 2)
Fit is performed for all hits up to 6 (the hits are chosen to maximize lever arm)
all selections uses number of layers, not number of hits
from the MTV below one can observe that

efficiency is not affected: even improves
there is a clear reduction of fakes
resolution dramatically improves
vertexing is not affected
My conclusion is that Physics performance improved.

MTV
ttbar: http://innocent.home.cern.ch/innocent/RelVal/gpuMTV122fbnew/
single muon ideal: http://innocent.home.cern.ch/innocent/RelVal/SingleMuFlatIdeal_gpu122fbnew/

This PR focuses on Physics performance. Once settled, computational performance and code cleaning can be addressed in a purely technical PR.

expect regression!

p.s. Some technicality needed to be improve anyhow

Slides for TRK-POG in https://indico.cern.ch/event/1098365/#3-new-fishbone

@slava77
Copy link
Contributor

slava77 commented Nov 30, 2021

@slava77 , bot did not find any DQM_* files (https://cmssdt.cern.ch/SDT/jenkins-artifacts/ib-baseline-tests/CMSSW_12_2_X_2021-11-29-2300/slc7_amd64_gcc900/-GenuineIntel/matrixgpu-results/) for the GPU workflows 11634.506,11634.512,11634.522 which are run for GPU baseline. that is the reason there is no wf_mapping.txt file.

I see, I'm guessing that it's because of #36167

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 1, 2021

-1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-bed46a/20882/summary.html
COMMIT: eaa9260
CMSSW: CMSSW_12_2_X_2021-11-29-2300/slc7_amd64_gcc900
Additional Tests: GPU
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/36215/20882/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: 4 differences found in the comparisons
  • DQMHistoTests: Total files compared: 41
  • DQMHistoTests: Total histograms compared: 3042214
  • DQMHistoTests: Total failures: 5
  • DQMHistoTests: Total nulls: 1
  • DQMHistoTests: Total successes: 3042186
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: -0.004 KiB( 40 files compared)
  • DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
  • Checked 175 log files, 37 edm output root files, 41 DQM output files
  • TriggerResults: no differences found

@fwyzard
Copy link
Contributor

fwyzard commented Dec 1, 2021

+heterogeneous

@jfernan2
Copy link
Contributor

jfernan2 commented Dec 1, 2021

+1

@VinInn
Copy link
Contributor Author

VinInn commented Dec 1, 2021

Does geometry require more info?

@cvuosalo
Copy link
Contributor

cvuosalo commented Dec 1, 2021

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 1, 2021

This pull request is fully signed and it will be integrated in one of the next master IBs (but tests are reportedly failing). This pull request will now be reviewed by the release team before it's merged. @perrotta, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

@perrotta
Copy link
Contributor

perrotta commented Dec 1, 2021

Differences only show up in the GPU workflow 11634.506 (2021 TTbar_14TeV Patatrack_PixelOnlyTripletsGPU)

Could anyone please remind me what that workflow is intended to? Is it a workflow meant to test some "development" work in GPU? Is it eventually foreseen a CPU counterpart of that code (or maybe it exists already, but it is not tested in a workflow)?

@VinInn
Copy link
Contributor Author

VinInn commented Dec 1, 2021

@perrotta this is fully expected.
It is not development work: is THE HLT production mode (already tested during the Beam Test). It runs on CPU as well (enough to run it on a machine w/o GPU)

@perrotta
Copy link
Contributor

perrotta commented Dec 1, 2021

@perrotta this is fully expected. It is not development work: is THE HLT production mode (already tested during the Beam Test). It runs on CPU as well (enough to run it on a machine w/o GPU)

Grazie @VinInn
And, just for my own education: why that "HLT production mode" is not implemented in any of the other "cpu" workflows, which do not show any difference here?

@VinInn
Copy link
Contributor Author

VinInn commented Dec 1, 2021

@perrotta , no idea. Ask trigger coordination. I do not think that we have an official Run3 HLT menu yet.

@perrotta
Copy link
Contributor

perrotta commented Dec 1, 2021

+1

@perrotta
Copy link
Contributor

perrotta commented Dec 1, 2021

merge

@fwyzard
Copy link
Contributor

fwyzard commented Dec 2, 2021

Hi Andrea,

And, just for my own education: why that "HLT production mode" is not implemented in any of the other "cpu" workflows, which do not show any difference here?

The Patatrack pixel reconstruction is used in two ways in the .501, .502, .505 and .506 workflows:

  • in step 2, the HLT is taken as-is from the ConfDB dump (which still uses the legacy pixel reconstruction) and customised via HLTrigger/Configuration/python/customizeHLTforPatatrack.py
  • in step 3, the configuration of the pixel-only RECO is generated directly from cmsDriver, based on the cfi/cff files in the release

customizeHLTforPatatrack is not used by any other workflows, so they would not be affected by the changes in this PR.

As for Vincenzo's comment

I do not think that we have an official Run3 HLT menu yet.

that is correct: the first draft of the Run-3 menu will be the outcome of the Trigger Reviews that are going on this week, and there's very little in release for the time being.

Specifically for the pixel and full tracking, the plan is

  • wait for 12.2.0-pre3 with this PR
  • Tracking POG will update the HLT menu (in ConfDB and then in release) with the CPU-only configuration that includes the Patatrack pixel tracks and fishbone cleaning
  • I will update the customizeHLTforPatatrack functions to use that as the starting point add support for offloading to GPUs

Once that is done, all workflows should use the new pixel reconstruction by default for the HLT, and .501/.502/.505/etc. will use it also for step 3. More details in a couple of weeks, I guess.

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.

9 participants