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
Update NHitCuts_byTrackAlgo for hgcalTrack #31681
Update NHitCuts_byTrackAlgo for hgcalTrack #31681
Conversation
…or GeneralTracksImporter.
The code-checks are being triggered in jenkins. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-31681/18819
|
A new Pull Request was created by @hatakeyamak (Kenichi Hatakeyama) for master. It involves the following packages: RecoParticleFlow/PFTracking @perrotta, @jpata, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
@cmsbuild please test |
The tests are being triggered in jenkins.
|
thanks @hatakeyamak ! |
+1 |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
Issue #16914 (since 2016!) reported that by "using the supposedly-correct value of 3 resulted in losing most endcap muons" From the validation plots provided in the PR description I see that in the far endcaps you probably don't loose "most" muons, but certainly a large part of them: Is this accepted/understood? Is it something that can be fixed in the follow-up PR? (Maybe the answer to this is in the "Remark" paragraph in the PR description, but I admit I could not follow it completely) |
The plots at the link are now updated (http://hep.baylor.edu/hatake/PFval/val_11_2_pre6_NHitCuts_byTrackAlgo/plots/) |
+1
|
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. @silviodonato, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2) |
New categories assigned: upgrade @kpedro88 you have been requested to review this Pull request/Issue and eventually sign? Thanks |
+upgrade |
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. @silviodonato, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
...to be consistent as that for GeneralTracksImporter.
PR description:
The sixth entry of
https://github.com/cms-sw/cmssw/blob/master/RecoParticleFlow/PFTracking/python/hgcalTrackCollection_cfi.py#L10
has been known as a nonsensical value. See: #16914.
I didn't really trace all the history, but I checked how the PF output changes when we set it to a "reasonable" value which is consistent as that used for GeneralTrackImporter, and I don't see anything alarming [0].
For simPFProducer outputs, after this NHitCuts_byTrackAlgo, I see slightly smaller neutral hadron entries of low pt [1] and a very minor change in PF electrons and photons [2]. I think this is because we are including more tracks (from muonSeededStepInOut/OutIn) in the hgcalTrack collection, and consequently more HGCalHE hits are associated to tracks.
Remarks: Right now, PF muons are produced from particleFlowTmpBarrel when they are RECO muons, even if the muons are in endcap [3], and simPFProducer skips [4] muons from reco muon collection. While ticl attempts to reconstruct these endcap muons, as expected. When we switch from simPFProducer to pfTICL, it's likely that we will have to do some duplicate removal between pfTICL and particleFlowTmpBarrel at least for muons. I am hoping to address this in a follow-up PR.
[1] http://hep.baylor.edu/hatake/PFval/val_11_2_pre6_NHitCuts_byTrackAlgo/plots/ZMM_ParticleFlow/PackedCandidates/neutralHadron/neutralHadronEta.pdf
[2] http://hep.baylor.edu/hatake/PFval/val_11_2_pre6_NHitCuts_byTrackAlgo/plots/ZMM_ParticleFlow/PackedCandidates/electron/electronEta.pdf
http://hep.baylor.edu/hatake/PFval/val_11_2_pre6_NHitCuts_byTrackAlgo/plots/ZMM_ParticleFlow/PackedCandidates/photon/photonEta.pdf
[3] because veto skips tracks with muonref: https://cmssdt.cern.ch/lxr/source/RecoParticleFlow/PFProducer/plugins/importers/GeneralTracksImporterWithVeto.cc#0133
[4] https://cmssdt.cern.ch/lxr/source/RecoParticleFlow/PFSimProducer/plugins/SimPFProducer.cc#0389
PR validation:
Ran PF validation on ZMM with and without PU:
[0] http://hep.baylor.edu/hatake/PFval/val_11_2_pre6_NHitCuts_byTrackAlgo/plots/
PF candidates stay almost identical, as discussed above. The "offset" plot is nearly identical:
http://hep.baylor.edu/hatake/PFval/val_11_2_pre6_NHitCuts_byTrackAlgo/plots/OffsetStacks/stack_ZMM_NHitCuts3_ref_vs_ZMM_ref.pdf
Since #16914 unexpectedly discussed about large reco muon changes, here we checked that slimmedMuons are not affected.
[5] http://hep.baylor.edu/hatake/PFval/val_11_2_pre6_NHitCuts_byTrackAlgo/SlimmedMuonEta_ZMMNoPU.pdf
[6] http://hep.baylor.edu/hatake/PFval/val_11_2_pre6_NHitCuts_byTrackAlgo/SlimmedMuonEta_ZMMPU.pdf
if this PR is a backport please specify the original PR and why you need to backport that PR:
This is not a backport.
Att: @bendavid @rovere @felicepantaleo @kpedro88