-
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
Mask old b-taggers and keep only DNN taggers with Era modifier for run3. #39808
Conversation
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-39808/32679
|
A new Pull Request was created by @yuanchao (Yuan CHAO) for master. It involves the following packages:
@cmsbuild, @mandrenguyen, @clacaputo can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
please test |
-1 Failed Tests: RelVals RelVals
|
Hi @yuanchao , what about JP and DeepJet ? |
@AlexDeMoor Thank you very much for the comment. Originally I was told that DNN taggers will be the only supported ones in Run 3. Surely I can add back the JP tagger in the era modifier. As to DeepJet, it's not in the original RecoBTag_cff.py configuration file. This PR doesn't touch that. |
@AlexDeMoor One more thing. Does JP depend on "JetTagComputerRecord" GT? |
assign alca |
New categories assigned: alca @yuanchao,@francescobrivio,@malbouis,@saumyaphor4252,@tvami,@ChrisMisan you have been requested to review this Pull request/Issue and eventually sign? Thanks |
Thank you for the information @yuanchao . Kindly ping @soureek and @johnalison . Do you have the answer about the GT ("Does JP depend on "JetTagComputerRecord" GT?") |
As mentioned by Daniel Bloch in the email thread, we don't require this. |
-1 Failed Tests: RelVals RelVals
|
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3763cc/29114/summary.html Comparison SummarySummary:
|
+alca
|
After looking at the last test (https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3763cc/29114/summary.html), it seems we have a lot of failings coming from the BTV HLT. I kindly ping @johnalison , @NiclasEich and @marco-link for their feedback on this part 👀 |
please test refreshing the results |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3763cc/29418/summary.html Comparison SummarySummary:
|
Hello, are we still waiting for feedback from BTV HLT? @cms-sw/btv-pog-l2 |
hi @mandrenguyen I had a private communication with Soureek et al, and they said: I'd say we should let this PR in, and then let BTV remove the remaining dependencies for the HLT folder, what do you think? |
+reconstruction |
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. @perrotta, @dpiparo, @rappoccio (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
PR description:
Regarding the AlCa issue #58 (cms-AlCaDB/AlCaTools#58), old b-tagging GT records are request to be removed (starting) from 12_1_X (and so on). Due to the dependency on "BTauGenericMVAJetTagComputerRcd" and "JetTagComputerRecord", the corresponding b-tagging computers need to be masked in run3. As DNN taggers will be the only supported ones in run3, this PR masks the old taggers with Era modifier and keeps only DNN taggers like DeepCSV and particleNet. (track counting and jet probability are kept for reference as requested)
PR validation:
PR tested in local with runTheMatrix.py processes. However, the standard run 2 validation sequences will fail due to missing b-tagger objects.
This PR is for run 3. Not a backport and no backport planed.