-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Enabling the JetCore+DeepCore2.2.1 hybrid with MaxCand=30 (JC30DC) #43339
Conversation
…th DeepCoreSeeding ProcModifiers
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-43339/37794
|
A new Pull Request was created by @bouchamaouihichem for master. It involves the following packages:
@cmsbuild, @jfernan2, @mandrenguyen can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
please test |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-4a3e84/35973/summary.html Comparison SummarySummary:
|
I think this should be tested with one of the
|
test parameters:
|
…re. Also updating to latest model JC20DC (MaxCand = 20)
please test |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-4a3e84/36008/summary.html Comparison SummarySummary:
|
@bouchamaouihichem could you please confirm if the changes introduced in the jets are as expected? https://tinyurl.com/yp38ebfl |
@@ -158,7 +158,7 @@ | |||
maxPtForLooperReconstruction = 0.0) | |||
jetCoreRegionalStepBarrelTrajectoryBuilder = RecoTracker.CkfPattern.GroupedCkfTrajectoryBuilder_cfi.GroupedCkfTrajectoryBuilderIterativeDefault.clone( | |||
trajectoryFilter = dict(refToPSet_ = 'jetCoreRegionalStepBarrelTrajectoryFilter'), | |||
maxCand = 50, | |||
maxCand = 30, |
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.
from a chat in the TRK mattermost thread, this change is not enough, another update is needed before this PR should be merged.
I am currently testing a fix and will update the PR (or make another one) soon |
Added fix after after testing. Without the DeepCore ProcModifier JetCore will run with MaxCand =50. When the DeepCore ProcModifier is used, JetCore will run with MaxCand = 30 along with DeepCore. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-43339/37911
|
Pull request #43339 was updated. @cmsbuild, @jfernan2, @mandrenguyen can you please check and sign again. |
please test |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-4a3e84/36104/summary.html Comparison SummarySummary:
|
as expected, the changes are only in the .17 (deep core) workflow, where more tracks appear with the @cms-sw/reconstruction-l2 |
+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. @antoniovilela, @rappoccio, @sextonkennedy (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
PR description:
PR validation:
Validation on workflow 11923 (high pT QCD PU) for Tracking performance and 11834 (inclusive TTBAR PU) for timing performance.