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

customizeHitRecoveryInGluedDetTkSeedsOnly #17807

Merged

Conversation

gpetruc
Copy link
Contributor

@gpetruc gpetruc commented Mar 7, 2017

Customize that switches on HIP mitigation only in tracker-seeded tracks, for the leacy re-reco of HIP-affected eras.

80X backport is in gpetruc:customizeHitRecoveryInGluedDetTkSeedsOnly

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 7, 2017

A new Pull Request was created by @gpetruc (Giovanni Petrucciani) for master.

It involves the following packages:

RecoTracker/Configuration

@perrotta, @cmsbuild, @slava77, @davidlange6 can you please review it and eventually sign? Thanks.
@ghellwig, @makortel, @felicepantaleo, @GiacomoSguazzoni, @rovere, @VinInn, @mschrode, @dgulhan this is something you requested to watch as well.
@Muzaffar, @davidlange6, @smuzaffar you are the release manager for this.

cms-bot commands are listed here #13028

@slava77
Copy link
Contributor

slava77 commented Mar 7, 2017

@gpetruc
For 91X I think that this should be a part of the HiPM era (tracker_apv_vfp30_2016)

@gpetruc gpetruc force-pushed the customizeHitRecoveryInGluedDetTkSeedsOnly_91X branch from 683801f to 9e55144 Compare March 8, 2017 11:14
@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 8, 2017

Pull request #17807 was updated. @perrotta, @cmsbuild, @slava77, @davidlange6 can you please check and sign again.

@gpetruc
Copy link
Contributor Author

gpetruc commented Mar 8, 2017

Ok, now it's era based

@slava77
Copy link
Contributor

slava77 commented Mar 8, 2017

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 8, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/18252/console Started: 2017/03/08 17:59

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 8, 2017

-1

Tested at: 9e55144

You can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-17807/18252/summary.html

I found follow errors while testing this PR

Failed tests: RelVals

  • RelVals:

When I ran the RelVals I found an error in the following worklfows:
10021.0 step4

runTheMatrix-results/10021.0_TenMuE_0_200+TenMuE_0_200_pythia8_2017_GenSimFull+DigiFull_2017+RecoFull_2017+ALCAFull_2017+HARVESTFull_2017/step4_TenMuE_0_200+TenMuE_0_200_pythia8_2017_GenSimFull+DigiFull_2017+RecoFull_2017+ALCAFull_2017+HARVESTFull_2017.log

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 8, 2017

Comparison not run due to runTheMatrix errors (RelVals and Igprof tests were also skipped)

@slava77
Copy link
Contributor

slava77 commented Mar 9, 2017

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 9, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/18294/console Started: 2017/03/09 17:09

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 9, 2017

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 9, 2017

Comparison job queued.

@slava77
Copy link
Contributor

slava77 commented Mar 9, 2017 via email

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 9, 2017

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

There are some workflows for which there are errors in the baseline:
25.0 step 5
The results for the comparisons for these workflows could be incomplete
This means most likely that the IB is having errors in the relvals.The error does NOT come from this pull request

@slava77
Copy link
Contributor

slava77 commented Mar 15, 2017

@gpetruc
please add a link to your slides in the PR description for better documentation.
Thank you.

@slava77
Copy link
Contributor

slava77 commented Mar 15, 2017

+1

for #17807 9e55144

  • implemented as described
  • jenkins tests pass and comparisons with baseline show small differences only in 136.731 (2016B workflow using HIPM era) as expected
  • higher stat test using 2016E (where the "HIP" effect is more significant) show expected reduction in the number of reconstructed muons, which appear to be predominantly bad quality.

looking at 136.76 (DoubleMuon 2016E)

as expected, there is some reduction in the number of muons (–3.7% in the tested file)

most of the removed muons didn't pass ID
in this example of TM lastStation angle tight ID there are actually more muons passing now
all_sign878vsorig_rundoublemuon2016ewf136p76c_booledmvaluemap_muons_muidtmlaststationangtight_rereco_obj_values_

This is more clearly supported with the standard muon eff DQM plots:
wf136 76_mueff_tight_eta

a few high-MET events went away
all_sign878vsorig_rundoublemuon2016ewf136p76c_log10recopfmets_pfmet__rereco_obj_sumet

@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 requires discussion in the ORP meeting before it's merged. @Muzaffar, @davidlange6, @smuzaffar

@davidlange6
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit dd3b138 into cms-sw:master Mar 16, 2017
@slava77
Copy link
Contributor

slava77 commented Apr 5, 2017

@gpetruc
we still need a PR for 80X
Please submit one at your earliest.
Thank you.

@gpetruc gpetruc deleted the customizeHitRecoveryInGluedDetTkSeedsOnly_91X branch March 8, 2021 09:10
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

4 participants