Ctagging 76 integration #11122

mverzett · 2015-09-04T09:51:48Z

First version of CTagging implementation
Adding @pvmulder and @imarches .

cmsbuild · 2015-09-04T09:56:12Z

A new Pull Request was created by @mverzett (Mauro Verzetti) for CMSSW_7_6_X.

Ctagging 76 integration

It involves the following packages:

DataFormats/BTauReco
PhysicsTools/PatAlgos
RecoBTag/CTagging
RecoBTag/Configuration
RecoBTag/SecondaryVertex
RecoBTau/JetTagComputer
RecoVertex/AdaptiveVertexFinder

The following packages do not have a category, yet:

RecoBTag/CTagging

@cmsbuild, @cvuosalo, @vadler, @monttj, @slava77 can you please review it and eventually sign? Thanks.
@TaiSakuma, @jdolen, @imarches, @makortel, @pvmulder, @acaudron, @mmarionncern, @GiacomoSguazzoni, @rappoccio, @rovere, @VinInn, @ahinzmann, @nhanvtran, @schoef, @ferencek, @gpetruc, @mariadalfonso, @cerati, @dgulhan this is something you requested to watch as well.
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.
If you are a L2 or a release manager you can ask for tests by saying 'please test' in the first line of a comment.
@Degano you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

slava77 · 2015-09-07T11:58:47Z

@cmsbuild please test

cmsbuild · 2015-09-07T12:01:18Z

The tests are being triggered in jenkins.

cmsbuild · 2015-09-07T14:16:49Z

-1

Tested at: bbe26d1
I found errors in the following unit tests:

---> test runtestTqafTopTools had ERRORS
---> test runtestTqafTopEventProducers had ERRORS
---> test runtestTqafTopKinFitter had ERRORS
---> test runtestTqafTopHitFit had ERRORS
---> test runtestTqafTopJetCombination had ERRORS
---> test runtestTqafTopEventSelection had ERRORS
---> test runtestTqafExamples had ERRORS
---> test runtestPhysicsToolsPatAlgos had ERRORS

you can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-11122/7923/summary.html

mverzett · 2015-09-07T16:53:16Z

@Degano, @slava77
this is really weird I tested it in CMSSW_7_6_X_2015-08-25-1100 and the matrix (runTheMatrix.py -l limited -i all) and unittests (scram b -r -j 12 runtests ) were running smoothly (logs here and here).

I see that now this build is not available anymore.

It looks like it's most likely a configuration problem. I still have to get the hang on the unscheduled running mode.

slava77 · 2015-09-07T18:30:35Z

Hi Mauro,

I suggest you start from a fresh release and pick up the code as it's known to github (git cms-merge-topic 11122)
I suspect some incomplete commits are in your test area.

ferencek · 2015-09-07T18:42:21Z

DataFormats/BTauReco/interface/TemplatedSoftLeptonTagInfo.h

@@ -163,8 +163,8 @@ TaggingVariableList TemplatedSoftLeptonTagInfo<REF>::taggingVariables(void) cons
    const SoftLeptonProperties & data = m_leptons[i].second;
    list.insert( TaggingVariable(btau::leptonQuality,  data.quality(SoftLeptonProperties::Quality::leptonId, false)), true );
    list.insert( TaggingVariable(btau::leptonQuality2, data.quality(SoftLeptonProperties::Quality::btagLeptonCands, false)), true );
-    list.insert( TaggingVariable(btau::trackSip2dSig,  data.sip2d),    true );
-    list.insert( TaggingVariable(btau::trackSip3dSig,  data.sip3d),    true );
+    list.insert( TaggingVariable(btau::trackSip2dSig,  data.sip2d),    true ); //These are correct! it turns out is just a bad naming convention


Please remove these comments. This (very old) inconsistency will be addressed in a separate PR (as announced in https://twiki.cern.ch/twiki/bin/view/CMS/CMSSW_7_6_0).

OK, I will remove the comments, but please be aware that data.sip[23]d do not map to the value but to the significance. This means that the TaggingVariableName is correct, is the object data member to be misleading

mverzett · 2015-09-07T21:34:16Z

Hi @slava77, at the moment I am in the CMSWeek with sub-par connectivity, to say the least. I will try these changes ASAP.

slava77 · 2015-09-07T21:40:58Z

On 9/7/15 4:34 PM, Mauro Verzetti wrote:

Hi @slava77 https://github.com/slava77, at the moment I am in the
CMSWeek with sub-par connectivity, to say the least. I will try these
changes ASAP.

OK.
I don't think we are in a hurry, if the target is just 76X.

—
Reply to this email directly or view it on GitHub
#11122 (comment).

mverzett · 2015-09-08T18:05:04Z

Indeed it seems that something went lost when packing the commits for the PR and removing the xml weight files from the log history.

I will work for fixing it ASAP

slava77 · 2015-09-10T19:32:33Z

RecoBTag/CTagging/interface/CharmTagger.h

+#include "RecoBTau/JetTagComputer/interface/JetTagComputerRecord.h"
+
+//DEBUGGING
+#include "TFile.h"


please remove the TFile and TNtuple
If really needed for debugging, hide them under ifdefs to be picked up by recompilation

Also, there is no way TFile nor TNtuple can be made thread safe. These must be removed.

mverzett · 2015-09-25T22:52:55Z

OK, done. Everything should be OK now, but if you are not in a rush I would test it once more, to make sure I did not mess up with git cherry-picking.

cmsbuild · 2015-09-25T22:56:51Z

Pull request #11122 was updated. @cmsbuild, @cvuosalo, @vadler, @monttj, @slava77 can you please check and sign again.

slava77 · 2015-09-26T15:25:14Z

@cmsbuild please test

@mverzett are you now done and also confident that it's all correct?

cmsbuild · 2015-09-26T15:26:21Z

The tests are being triggered in jenkins.

cmsbuild · 2015-09-26T18:02:06Z

+1
Tested at: de5dfdb
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-11122/8409/summary.html

cmsbuild · 2015-09-26T18:50:36Z

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-11122/8409/summary.html

mverzett · 2015-09-27T08:18:38Z

@slava77 @ferencek, I think everything is in place. The only remaining thing to do is to uncomment these two lines once RECO RelVals with CTagging are available.

I will be offline on holidays for the next 3 weeks, but I think 99% of what was needed is done.

slava77 · 2015-09-28T13:11:06Z

+1

for #11122 de5dfdb

compared to the previously signed revision (mverzett/cmssw@599a3d1...de5dfdb ): inclusiveCandidateSecondaryVerticesCvsL are added to the list of stored products and their tracks to the whitelist for packed candidates to contain track info details.
jenkins tests pass and comparisons with baseline show differences only in the miniAOD monitoring of packed candidates with stored track information (about 0.2-0.3% increase in the number of such candidates)
additionally tested locally in CMSSW_7_6_X_2015-09-22-1500:
- the new product inclusiveCandidateSecondaryVerticesCvsL vertex collection appears in RECO and in AOD as expected and takes up ~0.1% of AOD
- there is no significant change in CPU
- allocated memory is reduced by ~70 kB after the removal of the vpset copy
- compared to the previously signed revision, there are no changes in the added ctagging variables

slava77 · 2015-09-28T13:11:52Z

@monttj please check this PR
Thank you.

Ctagging 76 integration

ferencek · 2015-10-23T02:09:21Z

@mverzett, RelVals with CTagging are now available and I prepared a PR to add them to PAT tests (see #12061). Once merged, you can go ahead with the remaining updates. One thing you'll need to be careful about is the fact that CvsL IVF vertices are not stored in MiniAOD. So if someone wants to re-run c-tagging on top of MiniAOD, CvsL IVF sequence will have to be re-run. For AOD this will not be a problem. Hence, a few tweaks in jetTools.py will be needed.

cmsbuild added this to the Next CMSSW_7_6_X milestone Sep 4, 2015

cmsbuild added reconstruction-pending analysis-pending pending-signatures tests-pending orp-pending new-package-pending comparison-pending labels Sep 4, 2015

slava77 mentioned this pull request Sep 7, 2015

add RecoBTag/CTagging to reconstruction category cms-sw/cms-bot#538

Merged

cmsbuild added tests-started and removed tests-pending labels Sep 7, 2015

cmsbuild added tests-rejected and removed tests-started labels Sep 7, 2015

ferencek reviewed Sep 7, 2015
View reviewed changes

slava77 reviewed Sep 10, 2015
View reviewed changes

cmsbuild added reconstruction-pending analysis-pending pending-signatures tests-pending comparison-pending and removed analysis-approved comparison-available fully-signed reconstruction-approved tests-approved labels Sep 25, 2015

cmsbuild added tests-started and removed tests-pending labels Sep 26, 2015

cmsbuild added tests-approved and removed tests-started labels Sep 26, 2015

cmsbuild added comparison-available and removed comparison-pending labels Sep 26, 2015

cmsbuild added reconstruction-approved and removed reconstruction-pending labels Sep 28, 2015

davidlange6 added a commit that referenced this pull request Sep 28, 2015

Merge pull request #11122 from mverzett/ctagging_76Integration

a656216

Ctagging 76 integration

davidlange6 merged commit a656216 into cms-sw:CMSSW_7_6_X Sep 28, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ctagging 76 integration #11122

Ctagging 76 integration #11122

mverzett commented Sep 4, 2015

cmsbuild commented Sep 4, 2015

slava77 commented Sep 7, 2015

cmsbuild commented Sep 7, 2015

cmsbuild commented Sep 7, 2015

mverzett commented Sep 7, 2015

slava77 commented Sep 7, 2015

ferencek Sep 7, 2015

mverzett Sep 7, 2015

mverzett commented Sep 7, 2015

slava77 commented Sep 7, 2015

mverzett commented Sep 8, 2015

slava77 Sep 10, 2015

Dr15Jones Sep 11, 2015

mverzett commented Sep 25, 2015

cmsbuild commented Sep 25, 2015

slava77 commented Sep 26, 2015

cmsbuild commented Sep 26, 2015

cmsbuild commented Sep 26, 2015

cmsbuild commented Sep 26, 2015

mverzett commented Sep 27, 2015

slava77 commented Sep 28, 2015

slava77 commented Sep 28, 2015

ferencek commented Oct 23, 2015

Ctagging 76 integration #11122

Ctagging 76 integration #11122

Conversation

mverzett commented Sep 4, 2015

cmsbuild commented Sep 4, 2015

slava77 commented Sep 7, 2015

cmsbuild commented Sep 7, 2015

cmsbuild commented Sep 7, 2015

mverzett commented Sep 7, 2015

slava77 commented Sep 7, 2015

ferencek Sep 7, 2015

Choose a reason for hiding this comment

mverzett Sep 7, 2015

Choose a reason for hiding this comment

mverzett commented Sep 7, 2015

slava77 commented Sep 7, 2015

mverzett commented Sep 8, 2015

slava77 Sep 10, 2015

Choose a reason for hiding this comment

Dr15Jones Sep 11, 2015

Choose a reason for hiding this comment

mverzett commented Sep 25, 2015

cmsbuild commented Sep 25, 2015

slava77 commented Sep 26, 2015

cmsbuild commented Sep 26, 2015

cmsbuild commented Sep 26, 2015

cmsbuild commented Sep 26, 2015

mverzett commented Sep 27, 2015

slava77 commented Sep 28, 2015

slava77 commented Sep 28, 2015

ferencek commented Oct 23, 2015