add ctagging discriminators in the default PAT content, small improvement to test script

[mverzett]

Just one commit that was scheduled to happen once the AOD RelVals with ctagging were available. Tested with

PhysicsTools/PatAlgos/test//runAsyncTests.pl --all

and with runTheMatrix, no patological sign.

[cmsbuild]

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

add ctagging discriminators in the default PAT content, small improvement to test script

It involves the following packages:

PhysicsTools/PatAlgos
RecoBTag/CTagging

@cmsbuild, @cvuosalo, @vadler, @monttj, @slava77 can you please review it and eventually sign? Thanks.
@rappoccio, @imarches, @ahinzmann, @acaudron, @mmarionncern, @jdolen, @nhanvtran, @schoef, @ferencek, @gpetruc, @mariadalfonso, @pvmulder, @TaiSakuma 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' or '@cmsbuild, 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.

[cvuosalo]

@cmsbuild please test

[cmsbuild]

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/9500/console

[cmsbuild]

-1

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

---> test runtestPhysicsToolsPatAlgos had ERRORS

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

[cvuosalo]

@mverzett: The Jenkins unit test error seems to be caused by your change. Please try to fix the problem.

[slava77]

I think the error in the unit tests is valid/relevant: it is triggered by excessive import of b-tagging configuration in PAT: it's trying to re-reconstruct pfCombinedCvsLJetTags instead of reading it from the event.
IMHO, a better choice is to read what's in the AOD file (and cleanup the import/load of configuration in PAT); otherwise you'd need to make sure to redo the TagInfos in the MINIAOD sequence as well.

@gpetruc @arizzi what's the current intended default MINIAOD setup when reading AOD regarding b-tags?
(redo full b-tagging or read from AOD)

[cmsbuild]

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

[slava77]

@mverzett
if this is really needed in 76X, we better get a solution in the next days; after that the DR campaign in MC will start

[mverzett]

@slava77
Ok, I thought you wanted some feedback from the others first. I will start working on it right away.
Just one comment though.

running the tests with ./runtests.sh and cmsRun patTuple_addBTagging_cfg.py I can reproduce the error, but running ./runAsyncTests.pl as I was instructed to does not show anything problematic:

patTuple_addBTagging_cfg.py: done events 1/10, total time 285s, 44 output lines, no exceptions

I think this should be also fixed, or otherwise removed.

[cvuosalo]

@cmsbuild please test

[cmsbuild]

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/9646/console

[mverzett]

Given that #12565 has been merged, how should I/you/we proceed with this one?

[ferencek]

@mverzett, as David explained, we wait for a validation in 80X and if no problems are found, this PR can simply be merged as is.

[pvmulder]

@ferencek @davidlange6 Given that we have an opportunity to get it in the official miniAOD for the 76X reprocessing, it may be worth to merge it in 76X as soon as possible, avoiding error-prone additional recipes on top of miniAOD. We still have a few days. Is it clear how long the validation of 80X will take? I would just like to understand if there is a chance to get it in 76X in time or if there is no hope at all.
Also, could you clarify what type of problems to expect when adding a new tagger?

[ferencek]

Hi Petra, I'm not exactly sure how long will a 80X validation cycle take and whether it is strictly necessary given the fact that the 80X PR passes all the unit tests. Therefore, I do not expect any problems with the 76X PR. The formal procedure is to merge new features in an open release cycle first and then backport to earlier release cycles and this requirement has been met now.

[pvmulder]

@davidlange6 @slava77 do you agree that @mverzett backports the c-tagger to 76X miniAOD now?

[slava77]

Petra, does "backport" mean just continuing with this PR?
The 80X version #12565 was merged. So, sure, why not.
It will still have to pass the tests here in 76X.

[mverzett]

@slava77 , yes, we mean try to merge this PR. As you can see the last time you guys tested it we were getting DAS errors, definitely not our fault. Moreover, #12565 has exactly the same code in it and worked perfectly.

Can you please give it another try?

[slava77]

@cmsbuild please test

[cmsbuild]

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/9993/console

[cmsbuild]

+1
Tested at: d6494b0
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-12288/9993/summary.html

[cmsbuild]

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

[slava77]

+1

for #12288 d6494b0

• it runs in all tests now
• and there are no differences in reco products (or anything monitored in jenkins comparisons)

clean candidate for re-miniAOD

@gpetruc @arizzi

[mverzett]

@slava77 whose approval is missing? If I understood correctly there is only a day or two left to enter the final 7.6 miniAOD re-production.

[slava77]

it's more in the "orp-pending" stage now (general release building decision)

[davidlange6]

just to clarify "The formal procedure is to merge new features in an open release cycle first and then backport to earlier release cycles and this requirement has been met now." is not correct. a validation is needed (here we are being asked/agreed to skip it..)

[ferencek]

@davidlange6, thank you for the clarification.