-
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
[HGCAL trigger] Migration to new code structure - First pass #36337
Conversation
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36337/27085
|
A new Pull Request was created by @jbsauvan (Jean-Baptiste Sauvan) for master. It involves the following packages:
@cmsbuild, @AdrianoDee, @srimanob, @cecilecaillol, @rekovic can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
please test |
-1 Failed Tests: RelVals RelVals-INPUT CMS deprecated warnings: 3 CMS deprecated warnings found, see summary page for details. RelVals
RelVals-INPUT
|
It needs to be tested with cms-data/L1Trigger-L1THGCal#23 |
test parameters:
|
please test |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-5ba14e/21009/summary.html CMS deprecated warnings: 3 CMS deprecated warnings found, see summary page for details. Comparison SummarySummary:
|
Hello, are there comments on this PR? |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-5ba14e/22456/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: CMS deprecated warnings: 3 CMS deprecated warnings found, see summary page for details. Comparison Summary@slava77 comparisons for the following workflows were not done due to missing matrix map:
Summary:
|
@jbsauvan the differences have disappeared, so they originated from it. |
882a50c
to
fc77fa5
Compare
Thanks @AdrianoDee |
@jbsauvan seems a good plan. Triggering them again. |
Please test |
+l1 |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-5ba14e/22479/summary.html CMS deprecated warnings: 3 CMS deprecated warnings found, see summary page for details. Comparison Summary@slava77 comparisons for the following workflows were not done due to missing matrix map:
Summary:
|
Ok, now I don't see any difference still. I'm not completely understanding what has happened but this shows that the differences saw came from something unrelated to this PR. It's enough for me. |
+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, @qliphy (and backports should be raised in the release meeting by the corresponding L2) |
@AdrianoDee while I am perfectly fine with the previous explanation provided by @jbsauvan , I would like to correct you: the results of the last tests do show the same differences (probably only numerical fluctuations) in the L1T DQM for wf 38634.0 that we were discussing above in this thread. |
+1 |
@perrotta you are right. I overlooked sorry. We will keep an eye on the next step. (Still some DQM plots that showed differences have gone) |
PR description:
Associated internal PRs and reviews:
This PR depends on external cms-data/L1Trigger-L1THGCal#23
PR validation:
Private tests in
L1Trigger/L1THGCalUtilities/test
:Tested D49, D60, D68, D77, D86 workflows
@EmyrClement @portalesHEP FYI