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

Backport of Phase1 Tracking in FastSim to 94X #24624

Merged
merged 29 commits into from
Sep 27, 2018

Conversation

angirar
Copy link
Contributor

@angirar angirar commented Sep 22, 2018

Backport of PR #23363 to 94X release which modifies tracking in FastSim for the phase1 upgraded pixel tracker.
All the changes are consistent in the older release except for one extra commit 7fc2cc2. It was necessary because some tracking features (related to mostly "SeedingLayerSetsBuilder" class) were available only from 10X onwards. And so this commit is a way around those missing features, serving the same functionality overall.

@cmsbuild
Copy link
Contributor

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-24624/30658/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 27
  • DQMHistoTests: Total histograms compared: 2721493
  • DQMHistoTests: Total failures: 108
  • DQMHistoTests: Total nulls: 248
  • DQMHistoTests: Total successes: 2720975
  • DQMHistoTests: Total skipped: 162
  • DQMHistoTests: Total Missing objects: 0

@ssekmen
Copy link
Contributor

ssekmen commented Sep 26, 2018

+1
The indentation in RecoTracker/TkSeedingLayers/src/SeedingLayerSetsBuilder.cc does not get fixed in github although it appears fixed in Angira's code. I suggest we do not delay this PR further and look into this later.

@kpedro88
Copy link
Contributor

+upgrade

@slava77
Copy link
Contributor

slava77 commented Sep 26, 2018

+1

for #24624 fc90815

  • even though the PR description is lacking any details, this backport of Phase1 in FastSim of CMS #23363 appears to be functionally correct
  • jenkins tests pass and comparisons with the baseline show some differences only in the DQM fastsim plots including seedingLayerSet values. This is apparently expected due to reordering of the seedingLayer sets.

@angirar please add some details to the PR description to improve the release self-documentation.
Thank you.

@angirar
Copy link
Contributor Author

angirar commented Sep 27, 2018

@slava77 I have provided a description of this PR now, thanks!

@ssekmen
Copy link
Contributor

ssekmen commented Sep 27, 2018

@civanch , @zhenhu , could you please sign? There was only an indentation correction. Thanks.

@civanch
Copy link
Contributor

civanch commented Sep 27, 2018

+1

@zhenhu
Copy link
Contributor

zhenhu commented Sep 27, 2018

+1

@kpedro88
Copy link
Contributor

+1

@kpedro88
Copy link
Contributor

merge

@cmsbuild cmsbuild merged commit 44925ca into cms-sw:CMSSW_9_4_X Sep 27, 2018
@angirar
Copy link
Contributor Author

angirar commented Sep 28, 2018

Thank you everyone!

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

9 participants