forked from cms-sw/cmssw
-
Notifications
You must be signed in to change notification settings - Fork 0
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
Remove TrackBaseRef #2
Labels
Comments
jpavel
pushed a commit
that referenced
this issue
Mar 27, 2014
jpavel
pushed a commit
that referenced
this issue
May 4, 2014
jpavel
pushed a commit
that referenced
this issue
May 13, 2014
jpavel
pushed a commit
that referenced
this issue
Sep 24, 2014
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add the code in https://github.com/makortel/cmssw/commits/tauQualityCuts
Inform Vincenzo/HN
https://hypernews.cern.ch/HyperNews/CMS/get/recoDevelopment/1246/1/1/1/1.html
More info (3/3/14 11:07)
Hi Pavel and Christian,
I noticed that in 710pre3 ~6 % of RECO per-event memory allocations
come from PFRecoTauDiscriminationByIsolation::discriminate()
http://cms-service-sdtweb.web.cern.ch/cms-service-sdtweb/ptf/igprof/reco/710p3/slc5_amd64_gcc481/navigator/reco_ttbar13_pu40_06_total/65
mostly via RecoTauQualityCuts::filterCand() and getTrackRef() in
RecoTauQualityCuts.cc
http://cms-service-sdtweb.web.cern.ch/cms-service-sdtweb/ptf/igprof/reco/710p3/slc5_amd64_gcc481/navigator/reco_ttbar13_pu40_06_total/185
Time-wise RecoTauQualityCuts::filterCand() takes ~1.5 % of RECO time
http://cms-service-sdtweb.web.cern.ch/cms-service-sdtweb/ptf/igprof/reco/710p3/slc5_amd64_gcc481/navigator/reco_ttbar13_pu40_05_perf/153
(profiles made with 100 events of 13 TeV TTBar + 40 PU at 25 ns)
I don't know if you have already planned developments on
RecoTauQualityCuts, but I thought to share some thoughts.
The culprit seems to be the non-default constructor(s) of
reco::TrackBaseRef (=edm::RefToBasereco::Track). I see that you need
TrackBaseRef in order to use reco::Vertex::trackWeight(), so its
construction can not be just simply removed. I think that in principle
the Vertex::trackWeight() could be changed such that it wouldn't
require TrackBaseRef (it onle checks if two Refs point to the same
Track, which should essentially boil down to a check if the original
collection and the index of the object are the same), but that is to
be discussed elsewhere.
I tested a couple of improvement ideas
https://github.com/makortel/cmssw/compare/tauQualityCuts
First, I transformed the quality cut abstractions to simpler functions
makortel@489e8b1
(the nice abstractions were slightly overengineered to my eye, but
YMMV). This gave 7 % improvement in RecoTauQualityCuts::filterCand()
http://cms-service-sdtweb.web.cern.ch/cms-service-sdtweb/ptf/igprof/reco/710p3/slc5_amd64_gcc481/navigator/reco_ttbar13_pu40_tau_15_perf/160
while, of course, the memory allocations stayed the same
http://cms-service-sdtweb.web.cern.ch/cms-service-sdtweb/ptf/igprof/reco/710p3/slc5_amd64_gcc481/navigator/reco_ttbar13_pu40_tau_16_total/185
In addition, the heap size due to RecoTauQualityCuts decreased by ~50 kB,
of which ~45 kB comes via PFRecoTauDiscriminationByIsolation
before http://cms-service-sdtweb.web.cern.ch/cms-service-sdtweb/ptf/igprof/reco/710p3/slc5_amd64_gcc481/navigator/reco_ttbar13_pu40_06.100_live/1406
after http://cms-service-sdtweb.web.cern.ch/cms-service-sdtweb/ptf/igprof/reco/710p3/slc5_amd64_gcc481/navigator/reco_ttbar13_pu40_tau_16.100_live/1428
Then, as allowed by the step above, I changed the filterTrack() to a
template on the Ref type
makortel@f831118
This way the TrackBaseRef can be constructed only if the
Vertex::trackWeight() is called. Since the minTrackVertexWeight cut is
disabled in 710pre3, the improvements are large:
2.6x in time of RecoTauQualityCuts::filterCand()
http://cms-service-sdtweb.web.cern.ch/cms-service-sdtweb/ptf/igprof/reco/710p3/slc5_amd64_gcc481/navigator/reco_ttbar13_pu40_tau_25_perf/464
All allocations got removed from filterCand(); comparing
PFRecoTauDiscriminationByIsolation::discriminate() gives ~21x
improvement
before http://cms-service-sdtweb.web.cern.ch/cms-service-sdtweb/ptf/igprof/reco/710p3/slc5_amd64_gcc481/navigator/reco_ttbar13_pu40_tau_16_total/65
after http://cms-service-sdtweb.web.cern.ch/cms-service-sdtweb/ptf/igprof/reco/710p3/slc5_amd64_gcc481/navigator/reco_ttbar13_pu40_tau_26_total/101
Still, the discriminators would be the heaviest component of tau reco
http://cms-service-sdtweb.web.cern.ch/cms-service-sdtweb/ptf/igprof/reco/710p3/slc5_amd64_gcc481/navigator/reco_ttbar13_pu40_tau_25_perf/16
(~9 % slower than RecoTauProducer)
I think the cost could be further reduced by decreasing the number of
times RecoTauQualityCuts::filterCand() is called. Looking from the
RECO configuration there are 25 instances of
PFRecoTauDiscriminationByIsolation being run, categorized to several
isolation methods, each with a couple of working points. I see that in
the following methods the quality cuts are the same for all working
points:
while for the rest (_Isolation, *IsolationDBSumPtCorr) there are
differences in quality cuts between working points. In addition, the
methods
appear to share the quality cuts.
One way to reduce the calls would be to perform the PFCandidate
filtering only once for all customers sharing the quality cuts. E.g.
for cases where the only difference is a cut value (e.g.
maximumSumPtCut between Loose, Medium, and Tight WP's of
*CombinedIsolationDBSumPtCorr) I would imagine it would be
straightforward to calculate once the quantity (e.g. SumPt) and then
implement the working points as a cut on the value without calculating
it again. Other cases would probably need larger changes.
What do you think? Would such changes make sense from your point of
view?
Cheers,
Matti
The text was updated successfully, but these errors were encountered: