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

Clean up of btag sequence in standard reco + update of DQM #13098

Merged
merged 31 commits into from Feb 12, 2016

Conversation

pvmulder
Copy link
Contributor

Removed pfTrackCountingHighPurBJetTags, pfSimpleSecondaryVertexHighPurBJetTags, pfCombinedMVABJetTags, pfCombinedSecondaryVertexBJetTags, pfCombinedSecondaryVertexSoftLeptonBJetTags

Added pfSimpleInclusiveSecondaryVertexHighEffBJetTags

For DQM: added c-tag discriminators + input variables, clean up to monitor relevant taggers (e.g. add pfSimpleSecondaryVertexHighPurBJetTags + removal of irrelevant discriminators)

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @pvmulder (Petra Van Mulders) for CMSSW_8_0_X.

It involves the following packages:

DQMOffline/RecoB
PhysicsTools/PatAlgos
RecoBTag/Configuration
RecoBTag/SecondaryVertex
Validation/RecoB

@cvuosalo, @monttj, @cmsbuild, @deguio, @slava77, @vadler, @vanbesien, @davidlange6 can you please review it and eventually sign? Thanks.
@TaiSakuma, @imarches, @ahinzmann, @acaudron, @mmarionncern, @rappoccio, @jdolen, @nhanvtran, @schoef, @ferencek, @gpetruc, @mariadalfonso, @pvmulder, @rociovilar this is something you requested to watch as well.
@slava77, @Degano, @smuzaffar you are the release manager for this.

cms-bot commands are list here #13028

cms.InputTag("pfTrackCountingHighEffBJetTags"),
cms.InputTag("pfSimpleSecondaryVertexHighEffBJetTags"),
cms.InputTag("pfSimpleSecondaryVertexHighPurBJetTags"),
cms.InputTag("pfSimpleInclusiveSecondaryVertexHighEffBJetTags"),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now only remove taggers. pfSimpleInclusiveSecondaryVertexHighEffBJetTags should be added only once new RelVals become available and are propagated to the list of input files for PAT tests.

@deguio
Copy link
Contributor

deguio commented Jan 28, 2016

please test

@cmsbuild
Copy link
Contributor

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

@cmsbuild
Copy link
Contributor

-1

Tested at: 2ed110f
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-13098/10801/summary.html

@slava77
Copy link
Contributor

slava77 commented Jan 28, 2016

The unit test and the addOn test fail in

Looking for module label: pfSimpleInclusiveSecondaryVertexHighEffBJetTags

this is related to #13098 (diff) #13098 (comment)

if ( btppColour ) col[1] = 4 ;
if ( !btppColour ) lineStyle[1] = 2 ;
}
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cleanup commented out?

@slava77
Copy link
Contributor

slava77 commented Jan 28, 2016

@pvmulder
please remove the commented out old taggers.
If you think they may be occasionally useful,
may I suggest to add a customize function that would enable all of them back and put.
I don't find it particularly practical to have to go over n~5 files to uncomment and recover something that was commented out and may be needed for someone's study and then need to recompile.
A simple customization will enable the legacy taggers with much less effort on the user side.

@ferencek
Copy link
Contributor

Perhaps it would be best to completely remove all the code, including producers and jet tag computers, related to the old CSV, CSV+SL, and the old CMVA.

@cmsbuild
Copy link
Contributor

@ferencek
Copy link
Contributor

Also, don't forget to update PhysicsTools/PatAlgos/test/patTuple_addBTagging_cfg.py accordingly.

@acaudron
Copy link

acaudron commented Feb 9, 2016

So I manage to come back before the merging with:
git reset --hard commitnumber
and I rebase the branch locally with success so I'm remaking my test.
When a branch is rebased, do I need to push it again?
Thanks for the help.

@slava77
Copy link
Contributor

slava77 commented Feb 9, 2016

you pushed something already, since the PR is now updated.
If that's not the end, then yes, you will need to push again if there are more changes coming.

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 9, 2016

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

@acaudron
Copy link

acaudron commented Feb 9, 2016

ok it should be fin now, local tests are fine and the branch is rebased

@fabozzi
Copy link
Contributor

fabozzi commented Feb 9, 2016

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 9, 2016

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

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 9, 2016

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 9, 2016

@cvuosalo
Copy link
Contributor

cvuosalo commented Feb 9, 2016

+1

For #13098 a7f42b5

Second approval after minor clean-up of DQM code. New Jenkins test results are still OK, with the same expected changes as discussed in my first approval message.

@bbilin
Copy link
Contributor

bbilin commented Feb 10, 2016

Dear all,

Want to let you know that I cleaned the TOP-DQM package for the obsolete taggers and made a PR(# 13241)

Best regards

Bugra, as one of TOP/DQM-PVT contacts

@pvmulder
Copy link
Contributor Author

@PARGALI great!

@pvmulder
Copy link
Contributor Author

@deguio can you have a new look and sign off if ok?

@deguio
Copy link
Contributor

deguio commented Feb 12, 2016

+1

davidlange6 added a commit that referenced this pull request Feb 12, 2016
Clean up of btag sequence in standard reco + update of DQM
@davidlange6 davidlange6 merged commit 23763ec into cms-sw:CMSSW_8_0_X Feb 12, 2016
@ferencek ferencek deleted the bTagRecoAndDQMcleanup branch February 12, 2016 21:13
This was referenced Mar 11, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet