Skip to content

Conversation

@mandrenguyen
Copy link
Contributor

@mandrenguyen mandrenguyen commented Nov 29, 2020

PR description:

The heavy-ion version of the CSV tagger was introduced into 11_2 in #31660
A peak is observed at a discriminator value of 0.30755
This was tracked down to change made during the review of #31660 which changed the way jets with no secondary vertex are identified.
This PR reverts to the original logic, and these jets now take a value of -1, as they should.
11_2_X backport is #32331

PR validation:

Validated with wfs 140.5611 and 158.01

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-32326/20157

  • This PR adds an extra 12KB to repository

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @mandrenguyen (Matthew Nguyen) for master.

It involves the following packages:

RecoBTag/Combined

@perrotta, @jpata, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks.
@ferencek, @JyothsnaKomaragiri, @emilbols, @andrzejnovak, @smoortga this is something you requested to watch as well.
@silviodonato, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

@slava77
Copy link
Contributor

slava77 commented Nov 29, 2020

please test with workflow 140.5611

@perrotta
Copy link
Contributor

please test workflow 140.5611

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 29, 2020

The tests are being triggered in jenkins.
Test Parameters:

@cmsbuild
Copy link
Contributor

+1
Tested at: 075599b
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-8a93ba/11145/summary.html
CMSSW: CMSSW_11_3_X_2020-11-29-0000
SCRAM_ARCH: slc7_amd64_gcc900

@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-8a93ba/11145/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 11 differences found in the comparisons
  • Reco comparison had 1 failed jobs
  • DQMHistoTests: Total files compared: 35
  • DQMHistoTests: Total histograms compared: 2529593
  • DQMHistoTests: Total failures: 9
  • DQMHistoTests: Total nulls: 1
  • DQMHistoTests: Total successes: 2529561
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.004 KiB( 34 files compared)
  • DQMHistoSizes: changed ( 312.0 ): 0.004 KiB MessageLogger/Warnings
  • Checked 148 log files, 34 edm output root files, 35 DQM output files

@mandrenguyen
Copy link
Contributor Author

@mandrenguyen, I think that the following image taken from the jenkins comparisons for wf 158.01 actually shows the effect of the bug (in the baseline) and the fix provided by this PR:
image

Out of curiosity: what does the value 0.30755 represent for the discriminator (i.e. why did it end up as being the "default" value)?

Thanks for the comparison (and also the suggestions). It would appear that if we send the xml file a negative mass value it will always return the same value for the discriminator. I haven't dug into the file to understand why that happens, but at least it gives the same value every time, and not something random, which would have been harder to spot.

@perrotta
Copy link
Contributor

perrotta commented Dec 1, 2020

@mandrenguyen (just in case you forgot it): if you apply here the last commit as you did for the 10_2 backport of this PR, I can launch the tests

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 1, 2020

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-32326/20189

  • This PR adds an extra 12KB to repository

@perrotta
Copy link
Contributor

perrotta commented Dec 1, 2020

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 1, 2020

The tests are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 1, 2020

Pull request #32326 was updated. @perrotta, @jpata, @cmsbuild, @slava77 can you please check and sign again.

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 1, 2020

+1
Tested at: f727ad8
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-8a93ba/11203/summary.html
CMSSW: CMSSW_11_3_X_2020-11-30-1100
SCRAM_ARCH: slc7_amd64_gcc900

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 1, 2020

Comparison job queued.

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 1, 2020

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

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 10 differences found in the comparisons
  • DQMHistoTests: Total files compared: 35
  • DQMHistoTests: Total histograms compared: 2529593
  • DQMHistoTests: Total failures: 13
  • DQMHistoTests: Total nulls: 1
  • DQMHistoTests: Total successes: 2529557
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.004 KiB( 34 files compared)
  • DQMHistoSizes: changed ( 312.0 ): 0.004 KiB MessageLogger/Warnings
  • Checked 148 log files, 34 edm output root files, 35 DQM output files

@perrotta
Copy link
Contributor

perrotta commented Dec 1, 2020

+1

  • It fixes the default value for the disciminator as intended
  • Jenkins tests pass and show the effect of the fix in the HI miniAOD

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 1, 2020

This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @silviodonato, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

@qliphy
Copy link
Contributor

qliphy commented Dec 2, 2020

+1

@cmsbuild cmsbuild merged commit 0838466 into cms-sw:master Dec 2, 2020
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.

5 participants