Run3-sim47 Remove reference to trackermaterial in materials.xml #28313
Conversation
|
The code-checks are being triggered in jenkins. |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-28313/12552
|
|
A new Pull Request was created by @bsunanda for master. It involves the following packages: Geometry/CMSCommonData @civanch, @Dr15Jones, @cvuosalo, @ianna, @mdhildreth, @cmsbuild, @kpedro88 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
@cmsbuild Please test |
|
The tests are being triggered in jenkins. |
|
@ianna I am happy as long as materials.xml does not refer to trackermaterial |
One should move these materials from materials.xml to trackermaterial.xml and update all Tracker xml files that use them. Then update the payloads. Alternatively, you can use the new materials.xml version since it's not used in the payloads and can be modified: |
|
Comparison job queued. |
|
@ianna I shall modify the xml files to do that but can someone update the payloads. But I don't see why payloads to be created. Moving the definition from one file to another will not change the physical quantities |
|
Comparison is ready Comparison Summary:
|
|
@cvuosalo I see, so this has nothing to do with the material reorganization. In any case a factor of 10 in preshower volumes does not sound healthy, but it depends on the real use of these volumes. And I do not see them in Run2/Run3 xml geometry. In any case, if we update xmls we have unrelated changes already, and if we want to keep the capability to produce Run1 geometries when DDD has gone I think we need to make the update, right? Although I am not sure this update is specifically needed by DD4hep (@bsunanda could you please confirm?) |
|
Hi all, because we do not want change DB geometry for Run-1,2 can we sign this PR, which for today will change XML geometry for Run-3 and Phase-2 any. Any result for Run-1,2 will not be changed. We agreed to restore functionality of XML for Run1,2, when Run3 will be completed and DD4Hep migration will be done. The condition: XML for Run1,2 should be identical to DB Run1,2. Only after that we can drop old DD. |
|
@civanch What I understand you saying is that, since we are not planning to make new DB payloads for Runs 1-2 in the near future, we are then free to change the Runs 1-2 XML files. The problem of the Runs 1-2 XML files will be pushed back until after the DD4hep migration is completed. But the plan you mentioned concerns me. The stated condition "XML for Runs 1-2 should be identical to the DB payloads for Runs 1-2" is true for the legacy releases of CMSSW (5_3 and 10_6) but will pose significant problems for new versions of CMSSW. We will need to discuss it at the next Sim meeting. |
|
unhold |
|
+1 |
cmsbuild
added
fully-signed
geometry-approved
and removed
geometry-pending
pending-signatures
labels
|
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. @silviodonato, @dpiparo (and backports should be raised in the release meeting by the corresponding L2) |
|
hold
|
|
Pull request has been put on hold by @silviodonato |
|
If I understand correctly,
If this is correct, I think we can safety merge this PR. |
|
@silviodonato - you are correct. I'd suggest to merge this PR and upload the new DB payloads for all Runs (and update the Run3 GTs). The GT change will result in a baseline comparison differences. However, we know that it would be superficial at this point. Then we'd need to update the baseline. This would make sure we'd catch any further geometry changes resulting in a baseline comparison differences. |
|
I agree with Ianna. Probably, optimal would be to complete fixing of Run-3 geometry and after make GTs for Run-3? We may agree, at what pre-release we stop updating Run-3 geometry. In a mean time this PR may be merged. |
|
unhold |
|
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. @silviodonato, @dpiparo (and backports should be raised in the release meeting by the corresponding L2) |
|
+1 |
PR description:
Several cfi files use only materials.xml and not trackermaterial.xml. A couple of materials refer to trackermaterial.xml for elements which are identically defined in materials.xml. Removing the references will allow the cfi's which only used materials.xml to work without any warnings.
PR validation:
Tested with runTheMatrix.py for standard workflows
if this PR is a backport please specify the original PR:
Nothing special