DeepCSV and CSVv2 negative tagger bugfix

[emilbols]

A series of bugs that were discovered in the current implementation of the DeepCSV and CSVv2 negative tagger has been fixed in this PR. A description of the bugs and the negative taggers in general can be found in this presentation at the BTV meeting [1].

It was also discovered that in the method used for producing inclusive secondary vertices, there is a cut on the angle between the vertex flight direction and the direction of the track sum. This cut needs to be inverted for a negative tagger, so a vertexing sequence with correct cuts for a negative tagger has been introduced.

In the plot [2] the negative tagger of DeepCSV is shown. The negative tagger discriminator values are shown from -1 to 0, and the full tagger is shown from 0 to 1.

[1] https://indico.cern.ch/event/713923/contributions/2938152/attachments/1620844/2578707/NegativeTagger.pdf

[2]

[cmsbuild]

The code-checks are being triggered in jenkins.

[cmsbuild]

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-23206/4664

Code check has found code style and quality issues which could be resolved by applying a patch in https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-23206/4664/git-diff.patch
e.g. curl https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-23206/4664/git-diff.patch | patch -p1

You can run scram build code-checks to apply code checks directly

[cmsbuild]

The code-checks are being triggered in jenkins.

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-23206/4665

[cmsbuild]

A new Pull Request was created by @emilbols for master.

It involves the following packages:

PhysicsTools/PatAlgos
RecoBTag/Combined
RecoBTag/DeepFlavour
RecoBTag/SecondaryVertex
RecoVertex/AdaptiveVertexFinder

@perrotta, @monttj, @cmsbuild, @slava77, @gpetruc, @arizzi can you please review it and eventually sign? Thanks.
@TaiSakuma, @gouskos, @felicepantaleo, @rappoccio, @ahinzmann, @seemasharmafnal, @mmarionncern, @imarches, @makortel, @acaudron, @jdolen, @ferencek, @GiacomoSguazzoni, @rovere, @VinInn, @jdamgov, @nhanvtran, @gkasieczka, @schoef, @ebrondol, @dgulhan, @JyothsnaKomaragiri, @mverzett, @gpetruc, @mariadalfonso, @pvmulder this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

[cmsbuild]

Pull request #23206 was updated. @perrotta, @monttj, @cmsbuild, @slava77, @gpetruc, @arizzi can you please check and sign again.

[slava77]

On 5/29/18 10:11 PM, emilbols wrote: I have changed this PR so it just fixes the existing bugs in DeepCSV, so this PR no longer adds the deepflavour negative tagger and the new deepflavour training. I have also addressed the comments by @slava77 <https://github.com/slava77> that are still relevant to this PR after the refactoring. —

I indeed see no changes in RecoBtag/DeepFlavour code modified also by the double-B PR. Thank you. We'll be able to make more changes in parallel .

[ferencek]

Please note that having a set of commits that introduces new features followed by a set of commits that reverts them as part of a PR is generally a bad idea. It unnecessarily pollutes the official code history and on top of that makes rebasing potentially quite painful if merge conflicts appear anywhere along the commit history. With the latest changes the PR name and description should in principle also be updated in order not to be misleading.

[emilbols]

Updated the description and title of this PR to reflect the recent changes

[slava77]

@cmsbuild please test

[cmsbuild]

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/28325/console Started: 2018/05/31 15:12

[cmsbuild]

+1
Tested at: 2d46d43
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-23206/28325/summary.html

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
f519fc5
7f15f5b
You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-23206/28325/git-log-recent-commits
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-23206/28325/git-merge-result

[cmsbuild]

Comparison job queued.

[cmsbuild]

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

Comparison Summary:

• No significant changes to the logs found
• Reco comparison results: 0 differences found in the comparisons
• DQMHistoTests: Total files compared: 31
• DQMHistoTests: Total histograms compared: 2902471
• DQMHistoTests: Total failures: 2
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 2902279
• DQMHistoTests: Total skipped: 190
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.0 KiB( 30 files compared)
• Checked 128 log files, 14 edm output root files, 31 DQM output files

[slava77]

+1

for #23206 2d46d43

• bugfixes in negative taggers (not including fixes in DeepFlavour, due to pending overlap ordering choice wrt TensorFlow-based DeepDoubleB tagger integration #23033 ). Changes look reasonable. The updates in this PR do not affect taggers running in standard workflows.
• jenkins tests pass and comparisons with the baseline show no differences.

[fabiocos]

@emilbols @slava77 as far as I can see from my check, this PR and #23033 are technical independent, so the order in which they are merged does not matter. Could you please confirm?

[slava77]

On 6/1/18 2:22 AM, Fabio Cossutti wrote: @emilbols <https://github.com/emilbols> @slava77 <https://github.com/slava77> as far as I can see from my check, this PR and #23033 <#23033> are technical independent, so the order in which they are merged does not matter. Could you please confirm? —

Thank you for checking. I meant to say already that this is independent in the signoff message, but I was apparently not clear enough.

[fabiocos]

+1

[fabiocos]

merge