[DDD] Fix building of DDD GEM reco geometry DB payload, backport of #36869 #36870
Conversation
|
A new Pull Request was created by @cvuosalo (Carl Vuosalo) for CMSSW_12_2_X. It involves the following packages:
@civanch, @Dr15Jones, @makortel, @cvuosalo, @ianna, @mdhildreth, @cmsbuild, @AdrianoDee, @srimanob can you please review it and eventually sign? Thanks. cms-bot commands are listed here
|
|
@cmsbuild please test |
|
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-0b7355/22176/summary.html Comparison SummaryThe workflows 1001.0, 1000.0, 136.88811, 136.874, 136.8311, 136.793, 136.7611, 136.731, 4.22 have different files in step1_dasquery.log than the ones found in the baseline. You may want to check and retrigger the tests if necessary. You can check it in the "files" directory in the results of the comparisons Summary:
|
|
-1 Failed Tests: RelVals RelVals
Expand to see more relval errors ... |
|
type bug-fix |
|
backport of #36869 |
|
+1 |
|
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-0b7355/22190/summary.html Comparison SummarySummary:
|
|
Given the tests above, it is confirmed that the updated GT is really needed, and it will not be possible in 12_2_1 to switch from the previus and the new fixed Run3 geometry simply with a GT change |
|
@srimanob, please, comment |
|
+Upgrade |
cmsbuild
added
fully-signed
upgrade-approved
and removed
pending-signatures
upgrade-pending
labels
|
This pull request is fully signed and it will be integrated in one of the next CMSSW_12_2_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_12_3_X is complete. 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) |
|
@srimanob @civanch my comment in #36870 (comment) was just the summary of a check I made, trying to verify a statement made during the last ORP meeting, i.e. "we can backport the fixes needed for the updated Run3 geometry in 12_2_X, and then use the GT to decide whether to use the previous Run3 geometry or the fixed one". The outcome of the tests run in this PR (it could have been any other PR among the ones expected to be backported) is that once these PRs are backported it will not be possible to switch to the "old" Run3 geometry simply by reverting to the "old" GT payloads. The conclusion is that once these PR will get merged in 12_2 there will be no way to run the "old" Run3 geometry in 12_2_1. For me that's perfectly fine: once fixed the technical issues there is no reason to produce samples in 12_2_1 with the "old" Run3 geometry. But this is something that PPD should be aware of. Let me then add here @rappoccio and Kaori (sorry, I miss her github name), for their own information |
|
Yes there will be. One can use the old GT and use one of the modifier flag which says it is old geometry
…________________________________
From: Andrea Perrotta ***@***.***
Sent: 04 February 2022 09:42
To: cms-sw/cmssw
Cc: Sunanda Banerjee; Mention
Subject: Re: [cms-sw/cmssw] [DDD] Fix building of DDD GEM reco geometry DB payload, backport of #36869 (PR #36870)
@srimanob<https://github.com/srimanob> @civanch<https://github.com/civanch> my comment in #36870 (comment)<#36870 (comment)> was just the summary of a check I made, trying to verify a statement made during the last ORP meeting, i.e. "we can backport the fixes needed for the updated Run3 geometry in 12_2_X, and then use the GT to decide whether to use the previous Run3 geometry or the fixed one".
The outcome of the tests run in this PR (it could have been any other PR among the ones expected to be backported) is that once these PRs are backported it will not be possible to switch to the "old" Run3 geometry simply by reverting to the "old" GT payloads.
The conclusion is that once these PR will get merged in 12_2 there will be no way to run the "old" Run3 geometry in 12_2_1.
For me that's perfectly fine: once fixed the technical issues there is no reason to produce samples in 12_2_1 with the "old" Run3 geometry. But this is something that PPD should be aware of.
Let me then add here @rappoccio<https://github.com/rappoccio> and Kaori (sorry, I miss her github name), for their own information
—
Reply to this email directly, view it on GitHub<#36870 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABGMZOTXQCA2MLN42J7KLJ3UZOGQLANCNFSM5NNNHKYA>.
Triage notifications on the go with GitHub Mobile for iOS<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
Ah, great! Thank you @bsunanda |
|
+1 |
PR #36835 fixed the building of the GEM reco geometry for DD4hep, but it caused a problem for building the DDD GEM reco geometry DB payload. Technically, an unreliable DDFilteredView copy constructor had been used, which caused the original DDFilteredView to be changed when the copy was altered. The solution is to create a separate, identical DDFilteredView that can be iterated through without altering the original.
Note that this PR only affects the manual process of creating the DDD GEM reco geometry DB payload, which is done by an expert. This code is never used in any workflow.
PR validation:
A correct DDD GEM reco geometry DB payload was created successfully with this PR, whereas previously the creation program crashed.
if this PR is a backport please specify the original PR and why you need to backport that PR:
Original PR is #36869. For completeness and consistency, this PR could be backported to 12_2, but it is not strictly necessary. There shouldn't be a need to create geometry DB payloads in 12_2, since the latest 12_3 tags are the ones that should be used with 12_2_1.