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

Updated json for DeepCSV Phase 2 model #26072

Merged
merged 2 commits into from May 11, 2019
Merged

Updated json for DeepCSV Phase 2 model #26072

merged 2 commits into from May 11, 2019

Conversation

dseith
Copy link
Contributor

@dseith dseith commented Mar 5, 2019

A pull request with the new json for DeepCSV Phase 2 has been made to: https://github.com/cms-btv-pog/RecoBTag-Combined

Accordingly, the name of the json in this file is updated to match the now available Phase 2 json.

added correct json for PhaseII to pfDeepCSVJetTags_cfi.py
@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 5, 2019

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 5, 2019

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-26072/8630

  • This PR adds an extra 12KB to repository

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 5, 2019

A new Pull Request was created by @dseith (David Seith) for master.

It involves the following packages:

RecoBTag/Combined

@cmsbuild, @perrotta, @slava77 can you please review it and eventually sign? Thanks.
@imarches, @smoortga, @acaudron, @HeinerTholen, @JyothsnaKomaragiri, @mverzett, @ferencek, @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

@slava77
Copy link
Contributor

slava77 commented Mar 5, 2019

A pull request with the new json for DeepCSV Phase 2 has been made to: https://github.com/cms-btv-pog/RecoBTag-Combined

we need a data PR in the cms-data/RecoBTag-Combined repository

@dseith
Copy link
Contributor Author

dseith commented Mar 5, 2019

The PR is already there: cms-btv-pog/RecoBTag-Combined#3

@dseith
Copy link
Contributor Author

dseith commented Mar 5, 2019

Ah my bad
Its in the cms-btv-pog repo
I will make a pull request to the cms-data repo

@dseith
Copy link
Contributor Author

dseith commented Mar 5, 2019

I've now made the PR to cms-data/RecoBTag-Combined: cms-data/RecoBTag-Combined#20

@smuzaffar
Copy link
Contributor

please test with cms-sw/cmsdist#4752

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 5, 2019

The tests are being triggered in jenkins.
Using externals from cms-sw/cmsdist#4752
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/33425/console

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 6, 2019

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 6, 2019

Comparison job queued.

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 6, 2019

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

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 82 differences found in the comparisons
  • DQMHistoTests: Total files compared: 32
  • DQMHistoTests: Total histograms compared: 3114826
  • DQMHistoTests: Total failures: 102
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3114527
  • DQMHistoTests: Total skipped: 197
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 31 files compared)
  • Checked 133 log files, 14 edm output root files, 32 DQM output files

@cmsbuild
Copy link
Contributor

cmsbuild commented May 9, 2019

The tests are being triggered in jenkins.
Tested with other pull request(s) cms-data/RecoBTag-Combined#23
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/128/console Started: 2019/05/09 17:48

@cmsbuild
Copy link
Contributor

-1

Tested at: 3be52ce

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

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

I found follow errors while testing this PR

Failed tests: UnitTests

  • Unit Tests:

I found errors in the following unit tests:

---> test testConditionDatabase_2 had ERRORS

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

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

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

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 108 differences found in the comparisons
  • DQMHistoTests: Total files compared: 33
  • DQMHistoTests: Total histograms compared: 3211964
  • DQMHistoTests: Total failures: 129
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3211631
  • DQMHistoTests: Total skipped: 204
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 32 files compared)
  • Checked 137 log files, 14 edm output root files, 33 DQM output files

@slava77
Copy link
Contributor

slava77 commented May 10, 2019

@cmsbuild please test with cms-data/RecoBTag-Combined#23

@smuzaffar I see in the compile log
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-cf893c/128/build.log
that essentially a full build is done.
It is rather wasteful at least in this context.
Perhaps we can add some control words to ask for a light build.

@slava77
Copy link
Contributor

slava77 commented May 11, 2019

Compared to the previous #26072 (comment), it seems like we have some improvements.

Here are comparisons relative to the previous version of the training:
wf20034_966f953VSsign1068_deepCSV_bvl
wf20034_966f953VSsign1068_deepCSV_cvl
Both are showing some slight improvement.

Compared to the current baseline,
we are even better in the BvsAll b.v.l ROC
wf20034_966f953VSorig_deepCSV_bvl
and just about the same (not slightly worse as in the previous version) in the CvsL c.v.l ROC
wf20034_966f953VSorig_deepCSV_cvl_cvl

@slava77
Copy link
Contributor

slava77 commented May 11, 2019

+1

for #26072 3be52ce combined with cms-data/RecoBTag-Combined#23 966f953

  • code changes are in line with the PR description and the follow up review
  • jenkins tests pass and comparisons with the baseline show differences only phase-2 workflows in Btag plots with deepCSV discriminants
  • local tests with 1K events without PU show some improvement in performance Updated json for DeepCSV Phase 2 model #26072 (comment)

@fabiocos
Copy link
Contributor

@kpedro88 the ball is in your field now...

@kpedro88
Copy link
Contributor

+upgrade

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next master IBs (but tests are reportedly failing). This pull request will now be reviewed by the release team before it's merged. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2)

@fabiocos
Copy link
Contributor

+1

@fabiocos
Copy link
Contributor

merge

@cmsbuild cmsbuild merged commit 9cc73ff into cms-sw:master May 11, 2019
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

9 participants