[DD4hep] Revise order of materials to prevent delayed resolution of references #34974

cvuosalo · 2021-08-23T03:11:52Z

The order of materials in the DD4hep XML files ensures that each material does not reference other materials before they are defined, except in a few cases. DDCMS code does handle the delayed resolution of such references, but there are issues with the implementation. Since there are only a handful of materials that are in the wrong order, the easiest approach is to simply revise the order of these few materials so that they no longer are referenced before they are defined.

The DD4hep extended geometry description in the DB had the materials in an unspecified order (they were read from an std::unordered_map). This PR makes the simple code revision to put the materials in the same order as they are in the XML files. In this way, if the materials in the XML files are in the correct order, the materials list in the DB will have the same order. With the materials in the correct order, DD4hep with geometry from XML and DD4hep with geometry from the DB will have the exact same material budget and should show the same Tracker efficiencies. This change should eliminate the small discrepancies in Tracker efficiencies between DD4hep XML and DD4hep DB as seen in the DD4hep pre4 validation.

This PR goes along with PR #34927. After both PRs are merged, the 2021 geometry configuration files will need to be re-generated in a simple PR. After that, a new extended geometry description using all these changes will need to be created, uploaded to the DB, and added to a new Global Tag.

There will be two more follow-up PRs. Once the new DD4hep GT is created and the previous DD4hep GT is removed from use, then a PR that requires DD4hep materials to be in the proper order will be submitted. In this PR, an exception will be generated if any material is referenced before it is defined. This PR will ensure going forward that no future PR that puts materials in the wrong order can be merged. We can't submit this planned PR right now because it would break workflow 11634.912.

The second follow-up PR is for reduced material scenarios. The following files will need their material order revised just as was done in Geometry/TrackerCommonData/data/PhaseI/v3/pixbarmaterial.xml:

Geometry/TrackerCommonData/data/zeroMaterial/2021/v1/pixbarmaterial.xml
Geometry/TrackerCommonData/data/FlatMinus05Percent/2021/pixbarmaterial.xml
Geometry/TrackerCommonData/data/FlatMinus10Percent/2021/pixbarmaterial.xml
Geometry/TrackerCommonData/data/FlatPlus05Percent/2021/pixbarmaterial.xml
Geometry/TrackerCommonData/data/FlatPlus10Percent/2021/pixbarmaterial.xml

A reduced material PR with these changes, along with creation and uploading of DD4hep reduced material scenarios to the DB, is planned.

PR validation:

A test DDCMS parser that would reject undefined materials was used to check that this PR puts the materials lists for both XML and the extended geometry description intended for the DB in the correct order.

Backport

The code changes in this PR will be submitted as a 12_0 backport. The XML changes are not necessary because the PPS XML materials file in this PR is not even used in 12_0, and as for the pixbarmaterial.xml file, the single material that is in the wrong order is never actually used in a volume.

cmsbuild · 2021-08-23T03:17:06Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34974/24814

This PR adds an extra 64KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File Configuration/Geometry/python/dict2021Geometry.py modified in PR(s): Run3-gex89 Attempt to define unqiue names of materials everywhere #34927

Code check has found code style and quality issues which could be resolved by applying following patch(s)

code-format:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34974/24814/code-format.patch
e.g. curl https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34974/24814/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

cvuosalo · 2021-08-23T03:18:18Z

This PR does not show the changes to pixbarmaterial.xml since our rules require a new version of the file be created. As information to reviewers, the only change to the file is to move the definitions of Polyurethane and SupplyTubeEpoxy ahead of SupplyTubeConn3 so they are defined before they are referenced in the definition of SupplyTubeConn3.

Note that a new version of ppstrackerMaterials.xml is not needed because this file has not yet been uploaded to the DB.

cmsbuild · 2021-08-23T03:31:48Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34974/24815

This PR adds an extra 64KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File Configuration/Geometry/python/dict2021Geometry.py modified in PR(s): Run3-gex89 Attempt to define unqiue names of materials everywhere #34927

cmsbuild · 2021-08-23T03:32:07Z

A new Pull Request was created by @cvuosalo (Carl Vuosalo) for master.

It involves the following packages:

Configuration/Geometry (geometry, upgrade)
DetectorDescription/DDCMS (geometry)
DetectorDescription/OfflineDBLoader (geometry)
Geometry/TrackerCommonData (geometry)
Geometry/VeryForwardData (geometry)

@civanch, @Dr15Jones, @makortel, @cvuosalo, @ianna, @kpedro88, @cmsbuild, @AdrianoDee, @srimanob, @mdhildreth can you please review it and eventually sign? Thanks.
@vargasa, @fabferro, @jan-kaspar, @JanFSchulte, @VinInn, @Martin-Grunewald, @bsunanda, @mmusich, @ghugo83, @mtosi, @fabiocos, @slomeo, @venturia this is something you requested to watch as well.
@perrotta, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

cvuosalo · 2021-08-23T03:45:54Z

@cmsbuild please test

srimanob · 2021-08-23T06:20:46Z

Hi @cvuosalo
Thanks very much for the investigation and PRs. Do I understand correctly that with this PR and #34927 we can make a test sample with XML used in SIM and RECO, and make first comparison it with DDD.

cmsbuild · 2021-08-23T09:35:15Z

-1

Failed Tests: UnitTests
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-7eb9eb/17955/summary.html
COMMIT: 293e880
CMSSW: CMSSW_12_1_X_2021-08-22-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/34974/17955/install.sh to create a dev area with all the needed externals and cmssw changes.

Unit Tests

I found errors in the following unit tests:

---> test test2021Geometry had ERRORS

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 39
DQMHistoTests: Total histograms compared: 3000352
DQMHistoTests: Total failures: 0
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3000330
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 38 files compared)
Checked 165 log files, 37 edm output root files, 39 DQM output files
TriggerResults: no differences found

civanch · 2021-08-23T14:06:33Z

@cvuosalo , should test 2021 geometry be updated?

cmsbuild · 2021-08-23T22:16:20Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34974/24838

This PR adds an extra 64KB to repository

cmsbuild · 2021-08-23T22:16:41Z

Pull request #34974 was updated. @civanch, @Dr15Jones, @makortel, @cvuosalo, @ianna, @mdhildreth, @cmsbuild can you please check and sign again.

cvuosalo · 2021-08-23T22:17:53Z

@cmsbuild please test

cmsbuild · 2021-08-24T01:31:49Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-7eb9eb/17986/summary.html
COMMIT: 370f7fc
CMSSW: CMSSW_12_1_X_2021-08-23-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/34974/17986/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 39
DQMHistoTests: Total histograms compared: 3000352
DQMHistoTests: Total failures: 0
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3000330
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 38 files compared)
Checked 165 log files, 37 edm output root files, 39 DQM output files
TriggerResults: no differences found

civanch · 2021-08-24T07:49:09Z

+1

cmsbuild · 2021-08-24T07:49:35Z

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)

civanch · 2021-08-24T07:53:31Z

urgent

perrotta · 2021-08-24T08:42:41Z

+1

Let merge in the master, so that further development PR can be tested on top of it

[DD4hep] Revise order of materials, partial backport of #34974

cvuosalo added 3 commits August 20, 2021 19:02

Fix material ordering

b465e7d

Create new v3 of pixbarmaterial.xml

c865552

Update dictionary for new v3 pixbarmaterial.xml

d289fa9

cmsbuild added this to the CMSSW_12_1_X milestone Aug 23, 2021

cmsbuild added code-checks-pending geometry-pending orp-pending pending-signatures tests-pending upgrade-pending labels Aug 23, 2021

cmsbuild added code-checks-rejected and removed code-checks-pending labels Aug 23, 2021

Code formatting

293e880

cmsbuild added code-checks-pending and removed code-checks-rejected labels Aug 23, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Aug 23, 2021

cmsbuild added tests-started and removed tests-pending labels Aug 23, 2021

perrotta mentioned this pull request Aug 23, 2021

Run3-gex89 Attempt to define unqiue names of materials everywhere #34927

Merged

cmsbuild added tests-rejected and removed tests-started labels Aug 23, 2021

cmsbuild added code-checks-pending tests-pending and removed tests-rejected upgrade-pending code-checks-approved labels Aug 23, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Aug 23, 2021

cmsbuild added tests-started and removed tests-pending labels Aug 23, 2021

cmsbuild added tests-approved and removed tests-started labels Aug 24, 2021

cmsbuild removed the geometry-pending label Aug 24, 2021

cmsbuild added fully-signed geometry-approved and removed pending-signatures labels Aug 24, 2021

cmsbuild added the urgent label Aug 24, 2021

cmsbuild added orp-approved and removed orp-pending labels Aug 24, 2021

cmsbuild merged commit 5431961 into cms-sw:master Aug 24, 2021

cmsbuild mentioned this pull request Aug 24, 2021

Removed lexical_cast boost dependency in CondCore #34932

Merged

cmsbuild added a commit that referenced this pull request Aug 27, 2021

Merge pull request #34987 from cvuosalo/matOrdFix120

10323b3

[DD4hep] Revise order of materials, partial backport of #34974

cmsbuild mentioned this pull request Aug 27, 2021

DataFormats/FEDRawData UBSAN warning fixed #35039

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DD4hep] Revise order of materials to prevent delayed resolution of references #34974

[DD4hep] Revise order of materials to prevent delayed resolution of references #34974

cvuosalo commented Aug 23, 2021

cmsbuild commented Aug 23, 2021

cvuosalo commented Aug 23, 2021

cmsbuild commented Aug 23, 2021

cmsbuild commented Aug 23, 2021

cvuosalo commented Aug 23, 2021

srimanob commented Aug 23, 2021

cmsbuild commented Aug 23, 2021

civanch commented Aug 23, 2021

cmsbuild commented Aug 23, 2021

cmsbuild commented Aug 23, 2021

cvuosalo commented Aug 23, 2021

cmsbuild commented Aug 24, 2021

civanch commented Aug 24, 2021

cmsbuild commented Aug 24, 2021

civanch commented Aug 24, 2021

perrotta commented Aug 24, 2021

[DD4hep] Revise order of materials to prevent delayed resolution of references #34974

[DD4hep] Revise order of materials to prevent delayed resolution of references #34974

Conversation

cvuosalo commented Aug 23, 2021

PR validation:

Backport

cmsbuild commented Aug 23, 2021

cvuosalo commented Aug 23, 2021

cmsbuild commented Aug 23, 2021

cmsbuild commented Aug 23, 2021

cvuosalo commented Aug 23, 2021

srimanob commented Aug 23, 2021

cmsbuild commented Aug 23, 2021

Unit Tests

Comparison Summary

civanch commented Aug 23, 2021

cmsbuild commented Aug 23, 2021

cmsbuild commented Aug 23, 2021

cvuosalo commented Aug 23, 2021

cmsbuild commented Aug 24, 2021

Comparison Summary

civanch commented Aug 24, 2021

cmsbuild commented Aug 24, 2021

civanch commented Aug 24, 2021

perrotta commented Aug 24, 2021