71 x taus quality cuts improvement #2843

jpavel · 2014-03-13T11:38:14Z

Speeding up the execution of tau quality cuts (commits by @makortel)
Reducing inefficiency of anti-mu discriminators at high PU (authored by @veelken)
Clean up to follow cmssw guidelines (i.e. removing couts where possible)

- do not veto track segments in innermost CSC chamber - count number of muon stations containing matched segments rather than summing matched segments of different muons in the same muon station (in the old implementation a tau was failing the anti-muon Loose discriminator in case there are 2 reco::Muons each having 1 matched segment in the innermost CSC chamber. The new implementation requires that the matched segments are in at least 2 different chambers before the tau fails the anti-muon discriminator)

…tyCuts

… events

cmsbuild · 2014-03-13T11:43:20Z

A new Pull Request was created by @jpavel (Pavel Jez) for CMSSW_7_1_X.

71 x taus quality cuts improvement

It involves the following packages:

RecoTauTag/RecoTau

@nclopezo, @cmsbuild, @anton-a, @thspeer, @slava77, @Degano can you please review it and eventually sign? Thanks.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.
@nclopezo, @ktf you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

cmsbuild · 2014-03-14T08:42:31Z

+1
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-2843/478/summary.html

slava77 · 2014-03-14T16:29:11Z

RecoTauTag/RecoTau/src/AntiElectronIDMVA5GBR.cc

@@ -252,7 +252,7 @@ double AntiElectronIDMVA5GBR::MVAValue(Float_t TauEtaAtEcalEntrance,
 {

  if ( !isInitialized_ ) {
-    std::cout << "Error: AntiElectronMVA not properly initialized.\n";
+    edm::LogError("ClassNotInitialized") << "Error: AntiElectronMVA not properly initialized.\n";


If it's a configuration problem, an exception is more appropriate. Else we'll just keep running, which is probably not what's intended.

jpavel · 2014-03-16T09:35:42Z

Hi Slava, thanks for suggestions - all are implemented.

cmsbuild · 2014-03-16T09:38:20Z

Pull request #2843 was updated. @nclopezo, @cmsbuild, @anton-a, @thspeer, @slava77, @Degano can you please check and sign again.

slava77 · 2014-03-17T04:21:59Z

Hi Pavel,

What was changed on the physics side in this PR?
I see a difference in recoPFTauDiscriminator_hpsPFTauDiscriminationByMediumIsolation__

slava77 · 2014-03-17T04:24:29Z

nm, I see that changes are expected, as mentioned in the PR submission message

slava77 · 2014-03-17T04:32:00Z

Are there any plots describing changes in performance?
This would be helpful for keeping track of changes.
Please post them here or a link to some talk.

BTW, plots with discrimination by ChargedIsolation, by MuonRejection (2 and 3), by CombinedIsolationDBSumPtCorr, and by Isolation
are changed in some way. It's not obvious to me all are related to the declared change.

... what's more disturbing on a separate level is that nothing showed up in jenkins tests in
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-2843/478/summary.html

slava77 · 2014-03-17T04:53:45Z

RecoTauTag/RecoTau/src/RecoTauConstructor.cc

@@ -48,13 +48,12 @@
 }

 void RecoTauConstructor::addPFCand(Region region, ParticleType type, const PFCandidateRef& ref, bool skipAddToP4) {
-  //std::cout << "<RecoTauConstructor::addPFCand>:" << std::endl;
-  //std::cout << " region = " << region << ", type = " << type << ": Pt = " << ref->pt() << ", eta = " << ref->eta() << ", phi = " << ref->phi() << std::endl;
+  edm::LogInfo("TauConstructorAddPFCand") << " region = " << region << ", type = " << type << ": Pt = " << ref->pt() << ", eta = " << ref->eta() << ", phi = " << ref->phi();


these should be replaced as well.
I suggest you grep for LogInfo in all the code and change to LogDebug

cmsbuild · 2014-03-21T14:03:19Z

Pull request #2843 was updated. @nclopezo, @cmsbuild, @anton-a, @thspeer, @slava77, @Degano can you please check and sign again.

cmsbuild · 2014-03-21T14:30:34Z

+1
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-2843/603/summary.html

slava77 · 2014-03-21T15:27:27Z

Nice, I will check results later today (including the Vertex::trackWeight in #2894 for CPU performance comparison)

makortel · 2014-03-21T16:53:51Z

Hi Slava,

I'd actually expect no performance improvement in tau quality cut code with #2894, because AFAICT the vertex weight cut is currently disabled in RECO, and in this PR the reco::TrackBaseRef is constructed ("the heavy operation" that can be avoided with #2894) only if the cut is enabled.

jpavel · 2014-03-21T17:07:33Z

Hi Matti, this is not exactly true. Although reco::TrackBaseRef is avoided by quality cuts, it is still being used heavily by https://github.com/jpavel/cmssw/blob/71X_taus_QualityCutsImprovement/RecoTauTag/RecoTau/src/RecoTauVertexAssociator.cc where the weight of tracks in vertex is used to find the tau vertex.

makortel · 2014-03-21T17:35:01Z

Hi Pavel,

Thanks for the correction (I didn't look outside RecoTauQualityCuts.cc for the uses). However, in RecoTauVertexAssociator.cc you pointed, an intermediate reco::TrackBaseRef is actually constructed explicitly and therefore #2894 should not give any improvement in there either.

VinInn · 2014-03-21T17:45:21Z

I think that if reco::TrackBaseRef is never dereferenced one gets the speed-up anyhow.
I agree that all those reco::TrackBaseRef in RecoTauVertexAssociator.cc should be removed
in favor of templates (or directly with RefWithIndex)

jpavel · 2014-03-22T11:43:12Z

I agree - but it would work only after #2894 is merged, no? Otherwise the templates would not be defined in the code, or am I missing something?

slava77 · 2014-03-22T14:15:14Z

Results based on tests in CMSSW_7_1_X_2014-03-18-0200

only muon rejection related plots in tau DQM changed.

Here is one diff (red is new) that looked particularly large, just to check with you that they make sense.
Note that the background-like sample (qcd dijet wflow 38.0) doesn't have this large difference.

wf 20.0 (single muon pt=10, 2K events, no PU)

jpavel · 2014-03-22T14:49:48Z

Hi Slava, something like this is indeed expected - the aim of Christian's fix was to improve efficiency at larger eta regions, while the fake rate should not change dramatically. I will also let @veelken to comment on this plot.

veelken · 2014-03-22T15:04:31Z

Hi Slava,

the plot is the probability for muons to pass the discriminatorAgainstMuons3Loose, right ?
Some change in this discriminator is indeed expected (we modified the cuts for taus within the CSC acceptance, because we saw a large drop in tau ID efficiency at |eta| > 1.2 as function of pile-up)
Do you happen to have the corresponding plot for discriminatorAgainstMuons3Tight also ?

Cheers,

Christian

On Mar 22, 2014, at 3:49 PM, Pavel Jez wrote:

Hi Slava, something like this is indeed expected - the aim of Christian's fix was to improve efficiency at larger eta regions, while the fake rate should not change dramatically. I will also let @veelken to comment on this plot.

—
Reply to this email directly or view it on GitHub.

slava77 · 2014-03-23T07:24:52Z

Hi Christian,

The tight one doesn't have as much of an increase near eta ~2

slava77 · 2014-03-23T07:44:03Z

+1

for #2843 ca398bb
tested in CMSSW_7_1_X_2014-03-18-0200 (sign322 )

No regressions other than in the muon rejection, which are expected.
CPU in TauDiscriminationProducerBase went down by about a factor of 2

veelken · 2014-03-23T09:07:44Z

Hi Slava,

thank you very much for sharing the plot. It is good to see that the Tight discriminator changed much less.
Analyses that depend on the mu -> tau fake-rate being small use the Tight discriminator.
We will investigate what is causing the difference at abs(eta) ~ 1.

Thanks again,

Christian

On Mar 23, 2014, at 8:24 AM, slava77 wrote:

Hi Christian,

The tight one doesn't have as much of an increase near eta ~2

—
Reply to this email directly or view it on GitHub.

cmsbuild · 2014-03-23T09:19:16Z

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_1_X IBs unless changes (tests are also fine). @nclopezo, @ktf can you please take care of it?

71 x taus quality cuts improvement

veelken and others added 10 commits February 21, 2014 12:04

make fixed cone and shrinking cone tau producers in boostedTau tag

8309231

rebased to new integration build with PR cms-sw#2449 in

9fbb369

removing cout statements from RecoTau package

8a9a199

Move all cuts to single functions

489e8b1

Reduce constructions of reco::TrackBaseRef to minimum in RecoTauQuali…

f831118

…tyCuts

Merged tauQualityCuts from repository makortel

bba5439

Merge branch 'removeCout' into 71X_taus_QualityCutsImprovement

fb6c109

Fixed inefficiency of anti-muon2 discriminator at high eta in high PU…

2a8ba39

… events

typo fix

7175810

jpavel added pending-signatures labels Mar 13, 2014

jpavel added pending-signatures labels Mar 14, 2014

slava77 reviewed Mar 14, 2014
View reviewed changes

fixing printouts

6dc9476

jpavel added pending-signatures labels Mar 16, 2014

slava77 reviewed Mar 17, 2014
View reviewed changes

jpavel added pending-signatures labels Mar 21, 2014

jpavel added fully-signed labels Mar 23, 2014

davidlange6 added a commit that referenced this pull request Mar 23, 2014

Merge pull request #2843 from jpavel/71X_taus_QualityCutsImprovement

c070ce0

71 x taus quality cuts improvement

davidlange6 merged commit c070ce0 into cms-sw:CMSSW_7_1_X Mar 23, 2014

jpavel added a commit to jpavel/cmssw that referenced this pull request Apr 1, 2014

Fixes of ineffieciency of anti-mu discriminator as in cms-sw#2843

582b73f

jpavel mentioned this pull request Apr 1, 2014

Backport 71x fixes to 70x branch #3139

Closed

jpavel deleted the 71X_taus_QualityCutsImprovement branch May 23, 2014 10:57

jpavel mentioned this pull request Jul 16, 2014

Simplification of the tau quality cuts #4672

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

71 x taus quality cuts improvement #2843

71 x taus quality cuts improvement #2843

jpavel commented Mar 13, 2014

cmsbuild commented Mar 13, 2014

cmsbuild commented Mar 14, 2014

slava77 Mar 14, 2014

jpavel commented Mar 16, 2014

cmsbuild commented Mar 16, 2014

slava77 commented Mar 17, 2014

slava77 commented Mar 17, 2014

slava77 commented Mar 17, 2014

slava77 Mar 17, 2014

cmsbuild commented Mar 21, 2014

cmsbuild commented Mar 21, 2014

slava77 commented Mar 21, 2014

makortel commented Mar 21, 2014

jpavel commented Mar 21, 2014

makortel commented Mar 21, 2014

VinInn commented Mar 21, 2014

jpavel commented Mar 22, 2014

slava77 commented Mar 22, 2014

jpavel commented Mar 22, 2014

veelken commented Mar 22, 2014

slava77 commented Mar 23, 2014

slava77 commented Mar 23, 2014

veelken commented Mar 23, 2014

cmsbuild commented Mar 23, 2014

71 x taus quality cuts improvement #2843

71 x taus quality cuts improvement #2843

Conversation

jpavel commented Mar 13, 2014

cmsbuild commented Mar 13, 2014

cmsbuild commented Mar 14, 2014

slava77 Mar 14, 2014

Choose a reason for hiding this comment

jpavel commented Mar 16, 2014

cmsbuild commented Mar 16, 2014

slava77 commented Mar 17, 2014

slava77 commented Mar 17, 2014

slava77 commented Mar 17, 2014

slava77 Mar 17, 2014

Choose a reason for hiding this comment

cmsbuild commented Mar 21, 2014

cmsbuild commented Mar 21, 2014

slava77 commented Mar 21, 2014

makortel commented Mar 21, 2014

jpavel commented Mar 21, 2014

makortel commented Mar 21, 2014

VinInn commented Mar 21, 2014

jpavel commented Mar 22, 2014

slava77 commented Mar 22, 2014

jpavel commented Mar 22, 2014

veelken commented Mar 22, 2014

slava77 commented Mar 23, 2014

slava77 commented Mar 23, 2014

veelken commented Mar 23, 2014

cmsbuild commented Mar 23, 2014