Mask more old b-taggers for run3_common #40464
Conversation
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40464/33631
|
|
A new Pull Request was created by @francescobrivio for master. It involves the following packages:
@cmsbuild, @mandrenguyen, @clacaputo, @swertz, @vlimant can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
enable nano |
|
@cmsbuild , please test |
|
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-d73827/29867/summary.html Comparison SummarySummary:
NANO Comparison SummarySummary:
Nano size comparison Summary:
|
|
@cms-sw/reconstruction-l2 @cms-sw/xpog-l2 @cms-sw/btv-pog-l2 could you kindly review this PR and let me know in case you have comments/suggestions? This is currently blocking the merging of #40445 (which needs this PR), and in turn #40445 is blocking further developments of MC GTs for 2023. Thanks a lot! |
| ) | ||
| run3_common.toModify(softPFMuonComputer, | ||
| useCondDB = cms.bool(False), | ||
| ) |
There was a problem hiding this comment.
Just a suggestion, not a strict requirement (coming from BTV Reco):
To apply these changes to all the Computers which might be called somewhere, these modifications could be extended to also touch positiveSoftPFMuonComputer,negativeSoftPFMuonComputer,positiveSoftPFElectronComputer,positiveSoftPFElectronComputer in the exact same way. All six producers here use the same PSet, so they could be changed now as well in one go to also set useCondDB = cms.bool(False), even if these additional four producers are not responsible for any errors in the tests currently.
Otherwise the changes look fine.
There was a problem hiding this comment.
Hi @AnnikaStein thanks for the suggestion!
Sure I can extend the change to the other producers, but I have one question:
all of them are using the same PSets (softPF[Muon|Electron]Common), so wouldn't it be easier to modify directly the "Common" PSets? Or that's too risky because the Common ones might be imported somewhere else?
There was a problem hiding this comment.
Hi @francescobrivio
I totally agree it would be easier. My initial concern was exactly what you mentioned, with the risk that it might be imported somewhere else. But actually, using the cmssdt lxr and github search, I only found some place in HLT for Phase-2, where this PSet is defined as well, but I found no place where it gets directly imported. From that point of view, it should be safe to follow up with changing the PSet itself in another PR.
There was a problem hiding this comment.
hi @AnnikaStein given that from that on it's less of an AlCaDB business, I'm wondering if you could submit the PR? (If yes, please also tag us, just that we can follow it was done)
There was a problem hiding this comment.
Hi @tvami
Ok, will do!
|
hi @cms-sw/reconstruction-l2 @cms-sw/xpog-l2 may I suggest to merge this PR as it is, and follow up with an another PR to tidy up the rest of the computer as suggested by @AnnikaStein ? My reasoning is that we actually have another GT update in the queue, after #40445 is merged (which needs this to be merged first) |
|
+1 That's fine by me. |
|
type btv |
|
+reconstruction |
cmsbuild
added
fully-signed
reconstruction-approved
and removed
reconstruction-pending
pending-signatures
labels
|
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 |
francescobrivio
changed the title
PR description:
This PR is part of cms-AlCaDB/AlCaTools#58 to clean-up old b-tagging tags that are deprecated for Run3 (similar to #40043) and which are giving errors in the tests of #40445 (which actually removes the b-tagging tags from the Run3 MC GTs).
Namely:
SoftLeptonByMVAComputers_cff.pytheuseCondDBflag is set toFalsefor Run3 so that the BDT loads the weights from the xml and not from the CondDB tag.applySubstructure_cff.pyonly theDeepCSVtaggers are kept forpatJetsAK8PFPuppiSoftDropSubjetsandpatJetsAK8PuppiI'd like to ask @cms-sw/btv-pog-l2 and @AlexDeMoor (who were previously involved in the review of similar PRs) if they agree with the proposed solution, or they have a different suggestion?
FYI @cms-sw/alca-l2 @yuanchao
PR validation:
I have successfully run some of the wfs failing in #40445 (including the GTs proposed in that PR) with:
runTheMatrix.py -l 312.0,13234.0 -j9 --ibeosBackport:
Not a backport, no backport needed