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

DyT v2 on up to eta 1p2 #26778

Merged
merged 1 commit into from
May 17, 2019
Merged

Conversation

clacaputo
Copy link
Contributor

@clacaputo clacaputo commented May 14, 2019

PR description:

This PR activate the new threshold parameterization for the DyT algorithm (introduced in #26343 ) only for muons up to eta 1.2, while using the old DyT threshold for eta greater than 1.2.

This choice has been made after validating the new parameterization in all the eta range link

The PR changes the default flag for selecting the parameterization mode from False to True and modifies DynamicTruncation::correctThrByPAndEta adding:

std::set<dyt_utils::etaRegion> regionsToExclude = {dyt_utils::etaRegion::eta2p0, dyt_utils::etaRegion::eta2p2,  dyt_utils::etaRegion::eta2p4 };
if ( ! regionsToExclude.count(this->region) ) thr = parametricThreshold();

Parameters for dyt_utils::etaRegion::eta2p0, dyt_utils::etaRegion::eta2p2, dyt_utils::etaRegion::eta2p4 are left in for future development.

PR validation:

Validation has been performed using:

runTheMatrix.py --nThreads=4 -l 10809.0

output

10809.0_SingleMuPt1000+SingleMuPt1000_pythia8_2018_GenSimFull+DigiFull_2018+RecoFull_2018+HARVESTFull_2018+ALCAFull_2018+NanoFull_2018 Step0-PASSED Step1-PASSED Step2-PASSED Step3-PASSED Step4-PASSED Step5-PASSED  - time date Tue May 14 12:50:00 2019-date Tue May 14 12:18:38 2019; exit: 0 0 0 0 0 0
1 1 1 1 1 1 tests passed, 0 0 0 0 0 0 failed

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-26778/9776

  • This PR adds an extra 24KB to repository

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @clacaputo (Claudio Caputo) for master.

It involves the following packages:

RecoMuon/GlobalTrackingTools
RecoMuon/L3MuonProducer

@perrotta, @Martin-Grunewald, @cmsbuild, @fwyzard, @slava77 can you please review it and eventually sign? Thanks.
@bellan, @abbiendi, @jhgoh, @echapon, @calderona, @HuguesBrun, @folguera, @battibass, @trocino, @amagitte, @bachtis, @rociovilar this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

@slava77
Copy link
Contributor

slava77 commented May 14, 2019

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented May 14, 2019

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/207/console Started: 2019/05/14 16:07

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

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

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 215 differences found in the comparisons
  • DQMHistoTests: Total files compared: 33
  • DQMHistoTests: Total histograms compared: 3212004
  • DQMHistoTests: Total failures: 2912
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3208758
  • DQMHistoTests: Total skipped: 334
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 32 files compared)
  • Checked 137 log files, 14 edm output root files, 33 DQM output files

@perrotta
Copy link
Contributor

Validations shown at the muon POG meeting and at the general PPD meeting have demonstrated that this change has some effect on dyt muons in the barrel (where the tails in the distribution of the momenta resolutions decrease), which translates into an almost negligible effect on the reco muons once the high pt muon algo is chosen with TuneP.

In order to verify whether some change is ever propagated to other high level observables, I generated some 1500 singleMuonPt1000 events with wf 10809.0 (2018 setup).

The number of muons which change their direction is indeed quite small:
all_mini_testPR26778VSorig_SingleMuPt1000in2018wf10809p0c_patMuons_slimmedMuons__RECO_obj_eta

Differences in the reconstructed muon pt are slightly larger
all_mini_testPR26778VSorig_SingleMuPt1000in2018wf10809p0c_patMuons_slimmedMuons__RECO_obj_pt
(it is unfortunate that this plot cut off at 1 TeV, but the trend is clear)

Differences are more visible if one stick on the muons reconstructed with dyt, before the TuneP algo decides which reconstruction select for the final muon object:
all_testPR26778VSorig_SingleMuPt1000in2018wf10809p0c_log10recoTracks_tevMuons_dyt_RECO_obj_pt
all_testPR26778VSorig_SingleMuPt1000in2018wf10809p0c_minrecoTracks_tevMuons_dyt_RECO_obj_normalizedChi2,29
all_testPR26778VSorig_SingleMuPt1000in2018wf10809p0c_recoTracks_tevMuons_dyt_RECO_obj_ndof

MET as a whole get touched, but not in a significant way:
all_mini_testPR26778VSorig_SingleMuPt1000in2018wf10809p0c_patMETs_slimmedMETs__RECO_obj_phi
all_mini_testPR26778VSorig_SingleMuPt1000in2018wf10809p0c_patMETs_slimmedMETs__RECO_obj_pt

As weel as other objects, as Taus for example:
all_testPR26778VSorig_SingleMuPt1000in2018wf10809p0c_recoPFTaus_hpsPFTauProducer__RECO_obj_et

@perrotta
Copy link
Contributor

Overall, the quick validation above confirms what shown in the meetings: changing the fixed dyt threshold with a parameterized one in the barrel has some effect (better resolution due to the reduction of the tails) on dyt muon themselves; it reflects in an almost negligible effect on the final muons (after the TuneP choice) and all other overall event observables.

Given that the code changes in this PR are quite simple, the decision whether to accept it in the Ultra Legacy release must rely on physics considerations rather than on computing code ones.

@perrotta
Copy link
Contributor

+1

  • Code changes are minimal, and only meant to allow the new parameterization of the thresholds in the barrel (|eta|<1.2)
  • Muon pt resolution improved (tails reduced) for dyt muons in the barrel; only a small part of this improvement gets propagated to the finally TuneP selected muon
  • Muon POG supports this change

@perrotta
Copy link
Contributor

@clacaputo please prepare a 10_6 backport

@Martin-Grunewald
Copy link
Contributor

+1

@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. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2)

@fabiocos
Copy link
Contributor

+1

@clacaputo we need a backport of this PR in 10_6_X

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.

7 participants