-
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
Phase2-hgx327F Modify more scripts in Geometry/HGCalCommonData/test/python to retain minimum number of scripts useful for V16, V17 and TB setups #39928
Conversation
… minimum number of scripts useful for V16, V17 and TB setups
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-39928/32854
|
A new Pull Request was created by @bsunanda (Sunanda Banerjee) for master. It involves the following packages:
@civanch, @Dr15Jones, @bsunanda, @makortel, @ianna, @mdhildreth, @cmsbuild, @AdrianoDee, @srimanob can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
@cmsbuild Please test |
-1 Failed Tests: RelVals-INPUT RelVals-INPUTThe relvals timed out after 4 hours. Comparison SummarySummary:
|
@cmsbuild Please test |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-7c98e2/28654/summary.html Comparison SummarySummary:
|
+geometry |
@srimanob Please approve this |
+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) |
Uhm, I was supposing that #39920 should have already fixed the issue pointed out by Sal, but it was evidently not the case. As usual, some more information in the PR description fields would help better understanding what the PR is actually intended for. For exemple here it is written "modify more scripts in Geometry/HGCalCommonData/test/python", but looking at the files touched there is clearly much more than a few test scripts. |
Hi @perrotta @bsunanda @rappoccio |
Ok, got it: thank you @srimanob Still, the question to @bsunanda remains: please update the PR description, because here it is written "modify more scripts in Geometry/HGCalCommonData/test/python", but looking at the files touched there is clearly much more than a few test scripts. |
The root cause of these failures is the same as in issues #39480 and #39445 (and so far unknown). #39920 made the inconsistency (wherever it comes from) more visible by always throwing an exception instead of random crashes. The cause can not be caused by pileup, because the failing workflow(s) do not use any pileup. |
Sorry - I am a bit confused in all this. The PR's #39920 and #39921 (backport to 12_5_X) came because of a na observation by Chris that in some workflow, there is some crash because some limits are violated - now this happens in cases where a cell which is valid in run2 or 3 appears ina geometry setup of run4. This can happen either for signal or for pileup files (the second case is more likely). The best is to protect the code with an exit statement - tis is certainly a configuration error of some sort. The current PR is a clean up of HGCal test scripts which never apears in any production scenario.
…________________________________
From: Matti Kortelainen ***@***.***
Sent: 01 November 2022 14:55
To: cms-sw/cmssw
Cc: Sunanda Banerjee; Mention
Subject: Re: [cms-sw/cmssw] Phase2-hgx327F Modify more scripts in Geometry/HGCalCommonData/test/python to retain minimum number of scripts useful for V16, V17 and TB setups (PR #39928)
Hi @perrotta<https://github.com/perrotta> @bsunanda<https://github.com/bsunanda> @rappoccio<https://github.com/rappoccio> I am not sure why #39920<#39920> plays the game here. From comment in the PR, #39920 (comment)<#39920 (comment)>, it seems that it is not a bug fix. It uses to protect wrong PU file, which I understand that it should not happen it you use the proper PU file. That what I tried to summarize in #39920 (comment)<#39920 (comment)>. Something beyond that?
Ok, got it: thank you @srimanob<https://github.com/srimanob> So the error pointed out here above will stay there until the correct files for mixing events are used instead.
The root cause of these failures is the same as in issues #39480<#39480> and #39445<#39445>. #39920<#39920> made the inconsistency (wherever it comes from) more visible by always throwing an exception instead of random crashes. The cause can not be caused by pileup, because the failing workflow(s) do not use any pileup.
—
Reply to this email directly, view it on GitHub<#39928 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABGMZOU2YVFLGTOOWPS3SGTWGEONNANCNFSM6AAAAAARSH7GDY>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
I would propose to have a clarification of #39920 in git issue. If it is clear later, we can just close the issue. Sorry to say this, the PR description does not align with what is mentioned here now. |
+1
|
PR description:
Modify more scripts in Geometry/HGCalCommonData/test/python to retain minimum number of scripts useful for V16, V17 and TB setups
PR validation:
Test by running the scripts
If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for:
Nothing special