-
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
Phase-2 L1T e/g: new Composite Electron ID #41290
Phase-2 L1T e/g: new Composite Electron ID #41290
Conversation
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-41290/35066
|
A new Pull Request was created by @cerminar for master. It involves the following packages:
@epalencia, @cmsbuild, @AdrianoDee, @srimanob, @aloeliger, @cecilecaillol can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
test parameters: |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-41290/35161
|
Pull request #41290 was updated. @epalencia, @cmsbuild, @AdrianoDee, @srimanob, @aloeliger, @cecilecaillol can you please check and sign again. |
please test |
@@ -525,5 +580,9 @@ | |||
l1tLayer1HGCalNoTK, | |||
l1tLayer1HF, | |||
l1tLayer1, | |||
l1tLayer1EG | |||
l1tLayer1Extended, |
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.
@cerminar The failures in all the logs mention that this is not defined. Is this reliant on some Phase 2 development that is in a PR to the l1t-offline-branch, or one that is in a PR here to main CMSSW?
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.
The Layer1 from extended tracking comes in from #41279
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.
@gpetruc Okay. That PR has been outstanding for far too long and things are beginning to pile-up behind it. I will try to get in contact with upgrade to see if they have a review or are willing to sign at this stage.
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-41290/35572
|
Pull request #41290 was updated. @epalencia, @cmsbuild, @AdrianoDee, @srimanob, @aloeliger, @cecilecaillol can you please check and sign again. |
Please test |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-7ffb09/32675/summary.html The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
You can see more details here: Comparison SummaryThere are some workflows for which there are errors in the baseline: Summary:
|
+l1 |
+upgrade |
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:
This PR integrates the new Composite ID for TkElectrons in the endcap.
The default Layer-1 and Layer-2 collections will now run the new algorithm. New collections running the old algorithm are kept for physics validation purposes.
See Peter's talk @ Level-1 Trigger Weekly Meeting
The PR needs: cms-data/L1Trigger-Phase2L1ParticleFlow#6 to work (not sure how to handle this in the tests...)
PR validation:
A full set of validation plots is available at:
https://cerminar.web.cern.ch/cerminar/plots/V91-CMSSW12.5/
Barrel objects are untouched.
TkEm objects are untouched.