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

Update btag from DeepCSV to ParticleNet for BTV DQM #45077

Merged
merged 4 commits into from
Jun 12, 2024

Conversation

yeonsu108
Copy link
Contributor

PR description:

This PR aims to

  • remove un-used tags
  • update btag from DeepCSV to ParticleNet
    • paths and working points are also updated
  • add turnon plots (efficiency vs. [pt, eta. phi])
    • efficiency = offline jet matched with online / (offline jet matched with online) & (online discriminator > b-tag cut)

PR validation:

This PR passes the basic test procedure suggested in the CMSSW PR instructions

If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for:

Not a backport, but it should be backported to CMSSW_14_0_X

@cmsbuild
Copy link
Contributor

cmsbuild commented May 28, 2024

cms-bot internal usage

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-45077/40393

  • This PR adds an extra 40KB to repository

@cmsbuild
Copy link
Contributor

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

It involves the following packages:

  • DQMOffline/Trigger (dqm)

@tjavaid, @syuvivida, @antoniovagnerini, @rvenditti, @cmsbuild, @nothingface0 can you please review it and eventually sign? Thanks.
@trocino, @mtosi, @missirol, @cericeci, @HuguesBrun, @Fedespring, @rociovilar, @jhgoh this is something you requested to watch as well.
@rappoccio, @sextonkennedy, @antoniovilela you are the release manager for this.

cms-bot commands are listed here

@mmusich
Copy link
Contributor

mmusich commented May 28, 2024

@cmsbuild, please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-e133b6/39590/summary.html
COMMIT: 35eb03f
CMSSW: CMSSW_14_1_X_2024-05-28-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/45077/39590/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • You potentially added 673 lines to the logs
  • Reco comparison results: 20 differences found in the comparisons
  • DQMHistoTests: Total files compared: 48
  • DQMHistoTests: Total histograms compared: 3338862
  • DQMHistoTests: Total failures: 24
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3338818
  • DQMHistoTests: Total skipped: 20
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 29819.874 KiB( 47 files compared)
  • DQMHistoSizes: changed ( 12834.0,... ): 4969.979 KiB HLT/BTV
  • Checked 202 log files, 165 edm output root files, 48 DQM output files
  • TriggerResults: no differences found

@mmusich
Copy link
Contributor

mmusich commented May 28, 2024

You potentially added 673 lines to the logs

looks like this PR triggers a lot of additional warnings, e.g.:

22 Lines only in /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-e133b6/12834.0_TTbar_14TeV+2024/step3_TTbar_14TeV+2024.log
   %MSG-w BTagAndProbe:  BTagAndProbe:BTagAndProbe_0e1m DATETIME CEST  Run: 1 Event: 1
   shallow tag handle not valid, will skip event
   %MSG-w BTagAndProbe:  BTagAndProbe:BTagAndProbe_1e0m DATETIME CEST  Run: 1 Event: 1
   %MSG-w BTagAndProbe:  BTagAndProbe:BTagAndProbe_1e1m DATETIME CEST  Run: 1 Event: 1
   %MSG-w BTagAndProbe:  BTagAndProbe:BTagAndProbe_0e1m DATETIME CEST  Run: 1 Event: 2
   %MSG-w BTagAndProbe:  BTagAndProbe:BTagAndProbe_1e0m DATETIME CEST  Run: 1 Event: 2
   %MSG-w BTagAndProbe:  BTagAndProbe:BTagAndProbe_1e1m DATETIME CEST  Run: 1 Event: 2
   %MSG-w BTagAndProbe:  BTagAndProbe:BTagAndProbe_0e1m DATETIME CEST  Run: 1 Event: 3
   %MSG-w BTagAndProbe:  BTagAndProbe:BTagAndProbe_1e0m DATETIME CEST  Run: 1 Event: 3
   %MSG-w BTagAndProbe:  BTagAndProbe:BTagAndProbe_1e1m DATETIME CEST  Run: 1 Event: 3
   %MSG-w BTagAndProbe:  BTagAndProbe:BTagAndProbe_0e1m DATETIME CEST  Run: 1 Event: 4
   %MSG-w BTagAndProbe:  BTagAndProbe:BTagAndProbe_1e0m DATETIME CEST  Run: 1 Event: 4
   %MSG-w BTagAndProbe:  BTagAndProbe:BTagAndProbe_1e1m DATETIME CEST  Run: 1 Event: 4
   %MSG-w BTagAndProbe:  BTagAndProbe:BTagAndProbe_0e1m DATETIME CEST  Run: 1 Event: 5
   %MSG-w BTagAndProbe:  BTagAndProbe:BTagAndProbe_1e0m DATETIME CEST  Run: 1 Event: 5
   %MSG-w BTagAndProbe:  BTagAndProbe:BTagAndProbe_1e1m DATETIME CEST  Run: 1 Event: 5
   %MSG-w BTagAndProbe:  BTagAndProbe:BTagAndProbe_0e1m DATETIME CEST  Run: 1 Event: 6
   %MSG-w BTagAndProbe:  BTagAndProbe:BTagAndProbe_1e0m DATETIME CEST  Run: 1 Event: 6
   %MSG-w BTagAndProbe:  BTagAndProbe:BTagAndProbe_1e1m DATETIME CEST  Run: 1 Event: 6
   %MSG-w BTagAndProbe:  BTagAndProbe:BTagAndProbe_0e1m DATETIME CEST  Run: 1 Event: 7

@yeonsu108
Copy link
Contributor Author

yeonsu108 commented May 29, 2024

The warning is understood, and it is an expected situation. We are not interested in the events.

If a lot of warning messages are a problem, we can turn it off. What do you think?

In the meantime, the shallowTagInfo is outdated, will be updated in the next PR (not this one)

@mmusich
Copy link
Contributor

mmusich commented May 29, 2024

If a lot of warnings are a problem, we can turn it off. What do you think?

I think leaving lots of warnings active is not a good policy (will contribute in filling logs and warning streams, see also #41456). Turning it off is an option, but wouldn't it be better to fix it at the origin?

@yeonsu108
Copy link
Contributor Author

yeonsu108 commented May 29, 2024

Thanks for insight, I create a commit to turn off the warning

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-45077/40416

  • This PR adds an extra 44KB to repository

@cmsbuild
Copy link
Contributor

Pull request #45077 was updated. @cmsbuild, @nothingface0, @antoniovagnerini, @rvenditti, @tjavaid, @syuvivida can you please check and sign again.

@mtosi
Copy link
Contributor

mtosi commented Jun 5, 2024

please, test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 5, 2024

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-e133b6/39712/summary.html
COMMIT: 7c9e8b1
CMSSW: CMSSW_14_1_X_2024-06-05-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/45077/39712/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • You potentially removed 3 lines from the logs
  • Reco comparison results: 4 differences found in the comparisons
  • DQMHistoTests: Total files compared: 48
  • DQMHistoTests: Total histograms compared: 3338862
  • DQMHistoTests: Total failures: 18
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3338824
  • DQMHistoTests: Total skipped: 20
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 29819.874 KiB( 47 files compared)
  • DQMHistoSizes: changed ( 12834.0,... ): 4969.979 KiB HLT/BTV
  • Checked 202 log files, 165 edm output root files, 48 DQM output files
  • TriggerResults: no differences found

@tjavaid
Copy link

tjavaid commented Jun 7, 2024

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 7, 2024

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. @antoniovilela, @rappoccio, @sextonkennedy (and backports should be raised in the release meeting by the corresponding L2)

@rappoccio
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 738f13d into cms-sw:master Jun 12, 2024
11 checks passed
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.

6 participants