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
Revise densities & atomic weights for chemical elements to match Geant4 #29856
Conversation
The code-checks are being triggered in jenkins. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-29856/15408
|
A new Pull Request was created by @cvuosalo (Carl Vuosalo) for master. It involves the following packages: Geometry/CMSCommonData @civanch, @Dr15Jones, @makortel, @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. |
+1 |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
@cvuosalo , I would agree with fixes in many of materials, however, there are questions:
|
@fabiocos No, there has been no material budget test. Who could perform this test? I would need instructions. |
its not obvious to me why standard densities should be always used in the simulation. (or are these densities not actually used by g4?) |
|
@davidlange6 I think the density of elements that exist in pure form in the detector are used (e.g. copper). But if an element only occurs in compounds, then the elemental density is probably never used. |
@cvuosalo , because there is no change of results I suspect, that all what is changed is not used in baseline simulation. These material descriptions are historical. Even if many of that materials are not used in CMS directly better to keep new values coherent with Geant4. |
These updated materials are in v1 sub-directory, so do not break anything. |
+1 |
urgent |
We would like this PR to go into 11_1_pre8. |
I guess what you mean is that you are updating only the xml geometry and not the GT one so that there is no effect. That doesn’t seem like a good approach as it leaves surprises for later.
On May 19, 2020, at 5:35 PM, Vladimir Ivantchenko <notifications@github.com<mailto:notifications@github.com>> wrote:
These updated materials are in v1 sub-directory, so do not break anything.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#29856 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABGPFQ7PISBF4SPWREP5THDRSKRLHANCNFSM4NCSNUKA>.
|
@davidlange6 , it may be used or not for run-3 - old material.xml is not removed and we need to discuss what is optimal. This new file may be useful for Phase-2, why use very old values for Phase-2? At the same time, the modification is really small, there is no one big fix, so no significant change of results are expected. |
@davidlange6 This PR should be the last step of revising Run 3 geometry. There have been several geometry revisions in the last few weeks. We decided to bundle them into a single Global Tag update. With the merging of this PR and the closing of 11_1 to new development, I will create new 2021 geometry DB payloads and a new Global Tag. I have already done preliminary tests that show that there are no major differences between 2021 DB and 11_1 XML geometry. |
Its instead a question of showing that the new values are better. Should these not get added to the run 3 GT (as thats the geometry that got updated) to see their effect?
On May 19, 2020, at 5:54 PM, Vladimir Ivantchenko <notifications@github.com<mailto:notifications@github.com>> wrote:
@davidlange6<https://github.com/davidlange6> , it may be used or not for run-3 - old material.xml is not removed and we need to discuss what is optimal. This new file may be useful for Phase-2, why use very old values for Phase-2? At the same time, the modification is really small, there is no one big fix, so no significant change of results are expected.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#29856 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABGPFQ5MCV7BALGME6BDZA3RSKTTJANCNFSM4NCSNUKA>.
|
@davidlange6 I did test this PR checking the DQM and physics values from workflow 11634.0_TTbar_14TeV+TTbar_14TeV_TuneCP5_2021 using geometry XML and found no significant differences. After I create new geometry DB payloads for 2021, they will be tested, but those will be overall tests of all geometry changes since the previous geometry DB payloads were created, and not just this PR. |
Thanks - this approach makes sense.. Sounds like the PR for the new GT will come soon.
On May 19, 2020, at 6:59 PM, Carl Vuosalo <notifications@github.com<mailto:notifications@github.com>> wrote:
@davidlange6<https://github.com/davidlange6> I did test this PR checking the DQM and physics values from workflow 11634.0_TTbar_14TeV+TTbar_14TeV_TuneCP5_2021 using geometry XML and found no significant differences. After I create new geometry DB payloads for 2021, they will be tested, but those will be overall tests of all geometry changes since the previous geometry DB payloads were created, and not just this PR.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#29856 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABGPFQ6KLRYGT55XZTDYGPTRSK3HXANCNFSM4NCSNUKA>.
|
+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. @silviodonato, @dpiparo (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
materials.xml
has contained for many years some non-standard or nonsense values for the densities and atomic weights of certain chemical elements. To avoid any confusion, we are standardizing these values to match those used by Geant4. References for the Geant4 values are as follows.Atomic weights from NIST: https://www.nist.gov/pml/periodic-table-elements
Densities: http://geant4-userdoc.web.cern.ch/geant4-userdoc/UsersGuides/ForApplicationDeveloper/html/Appendix/materialNames.html
2021 DDD and DD4hep scenarios are updated to use the new version of
materials.xml
. Going forward, all new Run 3 and Phase 2 scenarios should use this new version.PR validation:
Test DB payloads were created with the new materials file and compared with existing DB payloads to check for errors.
Workflow 11634.0_TTbar_14TeV+TTbar_14TeV_TuneCP5_2021 was run using XML geometry with and without this PR. The DQM results show no significant differences.
No backport is needed.