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

Remove TrackBaseRef #2

Closed
jpavel opened this issue Mar 11, 2014 · 0 comments
Closed

Remove TrackBaseRef #2

jpavel opened this issue Mar 11, 2014 · 0 comments
Assignees

Comments

@jpavel
Copy link
Owner

jpavel commented Mar 11, 2014

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:

  • *ChargedIsolation
  • *CombinedIsolationDBSumPtCorr
  • Raw*IsolationDBSumPtCorr
  • *CombinedIsolationDBSumPtCorr3Hits
  • MVA3Isolation_IsoPtSum
    while for the rest (_Isolation, *IsolationDBSumPtCorr) there are
    differences in quality cuts between working points. In addition, the
    methods
  • _ChargedIsolation, *CombinedIsolationDBSumPtCorr, Raw_IsolationDBSumPtCorr
  • _CombinedIsolationDBSumPtCorr3Hits, MVA3Isolation_IsoPtSum
    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

@jpavel jpavel self-assigned this Mar 11, 2014
@jpavel jpavel closed this as completed Mar 11, 2014
jpavel added a commit that referenced this issue Mar 23, 2014
Merging highPt fixes into boostedTaus
jpavel pushed a commit that referenced this issue Mar 27, 2014
jpavel pushed a commit that referenced this issue Mar 27, 2014
jpavel pushed a commit that referenced this issue May 13, 2014
jpavel pushed a commit that referenced this issue May 22, 2014
Brian W testing pull request
jpavel pushed a commit that referenced this issue May 22, 2014
jpavel pushed a commit that referenced this issue Jul 28, 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
Projects
None yet
Development

No branches or pull requests

1 participant