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

modified for Phase2 DeepJet retraining model #44431

Merged
merged 2 commits into from
Mar 20, 2024
Merged

Conversation

hsinweihsia
Copy link

@hsinweihsia hsinweihsia commented Mar 15, 2024

PR description:

The BTV@HLT group has a Phase2 DeepJet retrained model.
The target for this retraining is to retrain DeepJet and recreate/Improve TDR performance.
The retraining online performance is compared to the TDR/offline model. Please see the details here and here.
This PR is associated with the Phase2 DeepJet retraining model integration PR.

PR validation:

The model was tested locally in CMSSW_13_1_0.
Follow the Phase 2 HLT simplified menu and edmConfigDump to get the configuration file and the HLT dump phase2_hlt.py, and in process.hltPfDeepFlavourJetTagsModEta2p4 and
process.hltPfDeepFlavourJetTags, change the model_path to retained model.
After running cmsRun phase2_hlt.py, the same warnings from InclusiveCandidateVertexFinder and EcalRecHitProducer are observed as running with RecoBTag/Combined/data/DeepFlavourV01_PhaseII/model.onnx, which is the default model.
Please see the details in the presentation.

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:

It'll have to be backported to CMSSW_14_0_X.

Before submitting your pull requests, make sure you followed this checklist:

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 15, 2024

cms-bot internal usage

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44431/39505

  • This PR adds an extra 28KB to repository

@cmsbuild
Copy link
Contributor

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

It involves the following packages:

  • HLTrigger/Configuration (hlt)

@Martin-Grunewald, @cmsbuild, @mmusich can you please review it and eventually sign? Thanks.
@rovere, @Martin-Grunewald, @SohamBhattacharya, @missirol, @silviodonato this is something you requested to watch as well.
@antoniovilela, @rappoccio, @sextonkennedy you are the release manager for this.

cms-bot commands are listed here

@@ -17,7 +17,7 @@
'input_5'
),
mightGet = cms.optional.untracked.vstring,
model_path = cms.FileInPath('RecoBTag/Combined/data/DeepFlavourV01_PhaseII/model.onnx'),
model_path = cms.FileInPath('RecoBTag/Combined/data/DDeepFlavourV02_PhaseII/DeepJet_retraining_phase2_new_inputs.onnx'),
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo? DDeep to Deep?

Copy link
Author

Choose a reason for hiding this comment

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

Sorry for the typo.
I just fixed it and git push back to the repository.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44431/39506

  • This PR adds an extra 32KB to repository

@cmsbuild
Copy link
Contributor

Pull request #44431 was updated. @mmusich, @Martin-Grunewald, @cmsbuild can you please check and sign again.

@mmusich
Copy link
Contributor

mmusich commented Mar 15, 2024

test parameters:

@mmusich
Copy link
Contributor

mmusich commented Mar 15, 2024

The BTV@HLT group has a Phase2 DeepJet retrained model.

@hsinweihsia please edit the PR description to link the study that motivated this update.
Please add also a description of the validation done and plans for a backport.
Thank you

@mmusich
Copy link
Contributor

mmusich commented Mar 16, 2024

@cmsbuild, please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-7e7784/38186/summary.html
COMMIT: e80e4d8
CMSSW: CMSSW_14_1_X_2024-03-15-2300/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/44431/38186/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

@mmusich
Copy link
Contributor

mmusich commented Mar 16, 2024

I have no reason to suspect issues with this PR (test results are technically fine), but unfortunately we don't seem to have any DQM / validation for the phase-2 b-tagging objects in the DQM matrix (see also issue #39362). @hsinweihsia is is something that BTV POG can work on to improve our monitoring capabilities ?
For sign-off I'll wait explicit green-light from TSG upgrade @rovere @SohamBhattacharya

@SohamBhattacharya
Copy link
Contributor

This PR looks fine from the hlt-upgrade pov.

@mmusich
Copy link
Contributor

mmusich commented Mar 18, 2024

+hlt

@cmsbuild
Copy link
Contributor

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. @sextonkennedy, @antoniovilela, @rappoccio (and backports should be raised in the release meeting by the corresponding L2)
Notice This PR was tested with additional Pull Request(s), please also merge them if necessary: cms-data/RecoBTag-Combined#56

@antoniovilela
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit a33c53a into cms-sw:master Mar 20, 2024
12 checks passed
@mmusich
Copy link
Contributor

mmusich commented Apr 3, 2024

@hsinweihsia @rovere @SohamBhattacharya

It'll have to be backported to CMSSW_14_0_X.

do you still foresee this to happen?

@mmusich
Copy link
Contributor

mmusich commented Apr 3, 2024

type btv

@cmsbuild cmsbuild added the btv label Apr 3, 2024
@srimanob
Copy link
Contributor

Re-ping @hsinweihsia @rovere @SohamBhattacharya
Do you still expect this in 14_0? Should I wait for it, or start relvals as soon as I have a new release. Thx.

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.

7 participants