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

Pfmuon fix 90x #17703

Merged
merged 5 commits into from Mar 22, 2017
Merged

Pfmuon fix 90x #17703

merged 5 commits into from Mar 22, 2017

Conversation

cmsbuild
Copy link
Contributor

@cmsbuild cmsbuild commented Mar 2, 2017

low tight and loose muon ID efficiency due to low pfmuon efficiency due to GeneralTracksImporterWithVeto killing pftracks
This issue was noticed in #16797
pfmuon temp fix - hgcal will fix later

fix rotation in me0geometry - in me0 even and odd chambers are not inverted

@calabria @lgray @pietverwilligen @kpedro88
Automatically ported from CMSSW_9_0_X #17648 (original by @jshlee).
Please wait for a new IB (12 to 24H) before requesting to test this PR.

@cmsbuild
Copy link
Contributor Author

cmsbuild commented Mar 2, 2017

A new Pull Request was created by @cmsbuild for master.

It involves the following packages:

Geometry/GEMGeometryBuilder
RecoParticleFlow/PFProducer

@civanch, @Dr15Jones, @cvuosalo, @ianna, @mdhildreth, @cmsbuild, @kpedro88, @slava77, @davidlange6 can you please review it and eventually sign? Thanks.
@ghellwig, @mmarionncern, @rafaellopesdesa, @lgray, @cbernet, @bachtis 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

@ianna
Copy link
Contributor

ianna commented Mar 2, 2017

please test

@cmsbuild
Copy link
Contributor Author

cmsbuild commented Mar 2, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/18053/console Started: 2017/03/02 14:57

@slava77
Copy link
Contributor

slava77 commented Mar 2, 2017

@cmsbuild please test

@cmsbuild
Copy link
Contributor Author

cmsbuild commented Mar 2, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/18071/console Started: 2017/03/02 15:52

@slava77
Copy link
Contributor

slava77 commented Mar 2, 2017

-1

I think this should be split in two (ME0 geometry removed from this PR)

@cmsbuild
Copy link
Contributor Author

cmsbuild commented Mar 2, 2017

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

@slava77
Copy link
Contributor

slava77 commented Mar 2, 2017

@cmsbuild please test

@cmsbuild
Copy link
Contributor Author

cmsbuild commented Mar 20, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/18565/console Started: 2017/03/20 19:58

@cmsbuild
Copy link
Contributor Author

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

@cmsbuild
Copy link
Contributor Author

@cmsbuild
Copy link
Contributor Author

Comparison job queued.

@cmsbuild
Copy link
Contributor Author

@slava77
Copy link
Contributor

slava77 commented Mar 21, 2017

In TenMuExtendedE 2023D4 wf 21211

all_sign873vsorig_tenmuextendede2023d4wf21211p0c_recopfcandidates_particleflow__reco_obj_eta43
all_sign873vsorig_tenmuextendede2023d4wf21211p0c_recopfcandidates_particleflow__reco_obj_eta57

There is an expected "reassignment" of muons from |eta|>2.5 to be charged hadrons.
There is also some reduction in pf muons in |eta|<2.5
Given the improvements in ID, I'm guessing that the reduction in pfMus is from some duplicates
wf21211_tigheff_eta

MET is still better in this sample, which reaffirms that the changes are overall in good direction for muons
wf21211_pf_gen_v_reco

In 23034 with PU200:
all_sign873vsorig_ttbar14tev2023d4timingpu200localwf23034p0c_recopfcandidates_particleflow__reco_obj_eta43
all_sign873vsorig_ttbar14tev2023d4timingpu200localwf23034p0c_recopfcandidates_particleflow__reco_obj_eta57
This seems somewhat in line with #17703 (comment)

jet response, MET, and b-tags appear to behave essentially the same in this sample with PU (areas where earlier iterations looked problematic).

Technical performance: timing total has a moderate increase of about 1% (from 609 to 618 s/event), coming out from quite a few ups and downs (like particleFlowBlock time up by 30% from 1.9 to 2.6 s/evt)

Memory is roughly unchanged with PU200 (there is an apparent increase of 15MB/core over 8GB/core in the test job).

@slava77
Copy link
Contributor

slava77 commented Mar 21, 2017

+1

for #17703 47865c8

  • code changes are roughly in line with the needed modifications here Pfmuon fix 90x #17703 (comment): in HGCAL-related reco the tracks related to muons are imported in a special way that doesn't kill muon PF ID
    • unfortunately due to auto-fwd port, this PR description can not be edited by the original submitter.
  • jenkins tests pass and comparisons show differences only in "C2 (HGCAL)"-related workflows D4 and D8 with changes related to PF muons and hadrons and downstream quantities
  • local tests confirm the expected behavior (see Pfmuon fix 90x #17703 (comment) and Pfmuon fix 90x #17703 (comment))

@slava77
Copy link
Contributor

slava77 commented Mar 21, 2017

release-note: recover PF muon efficiency in workflows with HGCAL

@kpedro88
Copy link
Contributor

+1

@cmsbuild
Copy link
Contributor Author

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 129152e into cms-sw:master Mar 22, 2017
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

6 participants