-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
New mvaTTH weights for Run 3 #46492
base: master
Are you sure you want to change the base?
New mvaTTH weights for Run 3 #46492
Conversation
cms-bot internal usage |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-46492/42348
|
A new Pull Request was created by @Cvico for master. It involves the following packages:
@cmsbuild, @ftorrresd, @hqucms can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
type egamma |
enable nano |
@cmsbuild, please test |
please abort |
test parameters: |
variables = _legacy_electron_BDT_variable | ||
) | ||
|
||
(run2_egamma_2016 | run2_egamma_2018).toModify( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Cvico I think it should be run2_egamma_2017
instead of run2_egamma_2016
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @hqucms. You are right, I've just pushed the fix. Thanks!
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-46492/42354 |
Pull request #46492 was updated. @cmsbuild, @ftorrresd, @hqucms can you please check and sign again. |
please test |
-1 Failed Tests: RelVals RelVals-INPUT RelVals-NANO RelVals
Expand to see more relval errors ...RelVals-INPUT
RelVals-NANO |
Hi, sorry for delayed action, I'm at a conference. I think the issue above was due to a wrong call to pfRelIso03 variable (which is new in the training). I've taken the correct definition from the config file. This is now in line with what I use to train. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-46492/42360 |
Pull request #46492 was updated. @cmsbuild, @ftorrresd, @hqucms can you please check and sign again. |
please test |
+1 Size: This PR adds an extra 24KB to repository Comparison SummarySummary:
NANO Comparison SummarySummary:
Nano size comparison Summary:
|
+1 |
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, @rappoccio, @mandrenguyen, @antoniovilela (and backports should be raised in the release meeting by the corresponding L2) |
PR description:
Brief description
Dear reviewers,
This PR is to introduce the newest training of the Electron mvaTTH in CMSSW. The training has been performed following what was done already done for Run 2, but using Run 3 MC samples for training and validation.
Context of the PR
The goal for this new training is to be used as a substitute of the current mvaTTH only in Run 3 MC production. That is: for Run 2 we still need to keep the current mvaTTH implementation.
What this PR includes
This change should only affect Run 3 production. How the mva is evaluated changes a little bit with respect to the previous version: mainly the order in which the variables are parsed to TMVA and also we have added a new additional variable which is
LepGood_pfRelIso03_all
.IMPORTANT REMARK
The training has been done considering a MiniAOD that has a buggy mvaNoISO variable. Instead of considering the buggy variable, the training has been done with mvaISO (and so it reflects in the config file, the last input variable to the MVA is mvaISO). This will change in the future most likely.
Validation
I've performed a local test running a simple
cmsRun
script to see that the mva is loaded and nanoAOD is in fact produced. Below is the cmsDriver I've used (in a cmssw-el8 container):cmsDriver.py --python_filename GEN-Run3Summer23NanoAODv12-00146_1_cfg.py --eventcontent NANOEDMAODSIM --customise Configuration/DataProcessing/Utils.addMonitoring --datatier NANOAODSIM --fileout file:GEN-Run3Summer23NanoAODv12-00146.root --conditions 130X_mcRun3_2023_realistic_v14 --step NANO --scenario pp --filein "dbs:/WZto3LNu_TuneCP5_13p6TeV_powheg-pythia8/Run3Summer23MiniAODv4-130X_mcRun3_2023_realistic_v14-v1/MINIAODSIM" --era Run3_2023 --no_exec --mc -n 1
Below there's also a plot comparing the performance of this training vs the current one.
Other comments
Please let me know if the proposal is reasonable, and if there's anything else I can do for testing. I'm adding the L2 electron conveners @RSalvatico @afiqaize so they can follow up the PR as well. Feel free to add any other electron convener. I'm also tagging the placeholder PR that Riccardo prepare in the links below (I prefered creating a new one).
Linked PRs