[DD4hep] Prohibit use of undefined materials #36496

cvuosalo · 2021-12-14T16:50:12Z

Our code allowed composite materials in the geometry to be defined out of order, but there was a subtle problem with the resolution of these materials. Also, it turned out that very few materials were defined out of order. It was decided to simply change the few cases of out-of-order material definitions rather than debug the complexities of supporting such definitions.

The re-ordering of material definitions was propagated to the DB geometry 1, so now we can change the code to simply prohibit out-of-order material definitions. This PR adds an exception if a material that was not yet defined is referenced in a geometry XML file.

The Phase 2 D77 scenario had one material that was defined out of order. This PR puts that definition in the correct order so D77 can be run with DD4hep.

A number of unit tests used obsolete materials files. This PR updates the versions of materials files in those tests so they don't generate the exception for out-of-order material definitions.

Going forward, the change in this PR should prevent the introduction into the geometry of any material definitions that are in the wrong order, provided any such geometry change is checked with DD4hep.

PR validation:

For Run 3, all materials are already defined in proper order, so this PR has no noticeable effect. A test was done with material definitions in the wrong order, and the exception was generated as desired.

With this PR, DD4hep Phase 2 D77 simulation runs successfully.

No backport is needed.

cmsbuild · 2021-12-14T16:55:58Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36496/27368

This PR adds an extra 48KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File Configuration/Geometry/python/dict2026Geometry.py modified in PR(s): Phase 2 tracker: add T28 and T29 (geometries with bricked pixels for IT sensor design studies) #36457

cmsbuild · 2021-12-14T16:56:21Z

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

It involves the following packages:

Configuration/Geometry (geometry, upgrade)
DetectorDescription/DDCMS (geometry)
Geometry/CMSCommonData (geometry, upgrade)

@civanch, @Dr15Jones, @makortel, @cvuosalo, @ianna, @mdhildreth, @cmsbuild, @AdrianoDee, @srimanob can you please review it and eventually sign? Thanks.
@missirol, @vargasa, @kpedro88, @Martin-Grunewald, @bsunanda, @fabiocos, @slomeo 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-12-14T16:59:09Z

@cmsbuild please test

civanch · 2021-12-14T17:06:12Z

@cvuosalo , I think it is a very good approach to use exception in this case.

makortel · 2021-12-14T18:31:06Z

DetectorDescription/DDCMS/plugins/dd4hep/DDDefinitions2Objects.cc

-      ns.context()->unresolvedMaterials[nam].emplace_back(
-          cms::DDParsingContext::CompositeMaterial(ns.prepend(fracname), fraction));
+      throw std::runtime_error("DD4CMS Exception -- Composite material \"" + fracname + "\" or \"" +
+                               ns.prepend(fracname) + "\" not yet defined.");


Could this be cms::Exception?

cmsbuild · 2021-12-14T19:58:49Z

-1

Failed Tests: UnitTests
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-eda0e3/21270/summary.html
COMMIT: 56a0f25
CMSSW: CMSSW_12_3_X_2021-12-14-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/36496/21270/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 testDD4hepDDSolid had ERRORS
---> test DetectorDescriptionDDCMSTestDriver had ERRORS
---> test GeometryTrackerCommonDataTestDriver had ERRORS
---> test GeometryMuonCommonDataTestDriver had ERRORS
and more ...

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 4 differences found in the comparisons
DQMHistoTests: Total files compared: 42
DQMHistoTests: Total histograms compared: 3250719
DQMHistoTests: Total failures: 6
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3250691
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 41 files compared)
Checked 177 log files, 37 edm output root files, 42 DQM output files
TriggerResults: no differences found

cmsbuild · 2021-12-15T01:17:03Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36496/27376

This PR adds an extra 60KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File Configuration/Geometry/python/dict2026Geometry.py modified in PR(s): Phase 2 tracker: add T28 and T29 (geometries with bricked pixels for IT sensor design studies) #36457

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-36496/27376/code-format.patch
e.g. curl -k https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36496/27376/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

cmsbuild · 2021-12-15T21:32:18Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36496/27395

This PR adds an extra 60KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File Configuration/Geometry/python/dict2026Geometry.py modified in PR(s): Phase 2 tracker: add T28 and T29 (geometries with bricked pixels for IT sensor design studies) #36457

cmsbuild · 2021-12-15T21:32:44Z

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

cvuosalo · 2021-12-15T21:33:03Z

Geometry/TrackerCommonData/data/cms-test-ddpixbarlayerupgrade-algorithm.xml

-    <Include ref="Geometry/TrackerCommonData/data/testDDPixBarLayerUpgradeAlgorithm.xml"/>
-    <Include ref="Geometry/CMSCommonData/data/materials/2015/v1/materials.xml"/>
-    <Include ref="Geometry/TrackerCommonData/data/trackermaterial.xml"/>
+    <Include ref="Geometry/CMSCommonData/data/materials/2021/v1/materials.xml"/>


In this test data file, v1 of materials.xml needed to be used instead of the more recent v3.

cvuosalo · 2021-12-15T21:46:12Z

@cmsbuild please test

cmsbuild · 2021-12-16T01:23:01Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-eda0e3/21297/summary.html
COMMIT: 441da8b
CMSSW: CMSSW_12_3_X_2021-12-15-1100/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/36496/21297/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: 42
DQMHistoTests: Total histograms compared: 3250719
DQMHistoTests: Total failures: 0
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3250697
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 41 files compared)
Checked 177 log files, 37 edm output root files, 42 DQM output files
TriggerResults: no differences found

civanch · 2021-12-16T06:45:39Z

+1

comments are addressed

civanch · 2021-12-17T08:48:49Z

@srimanob, can you, please, review this?

srimanob · 2021-12-17T09:07:12Z

+Upgrade

Technical PR to handle material of DD4hep. No change is expected from this PR. The PR test runs fine.

cmsbuild · 2021-12-17T09:07: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)

perrotta · 2021-12-17T16:49:47Z

+1

Technical

qliphy · 2021-12-18T01:49:55Z

@cvuosalo

There are several unit test errors appearing after this PR merged. Could you have a check?

https://cmssdt.cern.ch/SDT/cgi-bin/logreader/slc7_amd64_gcc10/CMSSW_12_3_X_2021-12-17-2300/unitTestLogs/Geometry/TrackerGeometryBuilder#/54-54
https://cmssdt.cern.ch/SDT/cgi-bin/logreader/slc7_amd64_gcc10/CMSSW_12_3_X_2021-12-17-2300/unitTestLogs/MagneticField/Engine#/68-68
https://cmssdt.cern.ch/SDT/cgi-bin/logreader/slc7_amd64_gcc10/CMSSW_12_3_X_2021-12-17-2300/unitTestLogs/MagneticField/GeomBuilder#/44-44

dd4hep: Error interpreting XML nodes of type
dd4hep: Error interpreting XML nodes of type
----- Begin Fatal Exception 18-Dec-2021 01:38:55 CET-----------------------
An exception of category 'StdException' occurred while
[0] Processing Event run: 1 lumi: 1 event: 1 stream: 0
[1] Running path 'p1'
[2] Prefetching for module testMagneticField/'testMagneticField'
[3] Prefetching for EventSetup module DD4hep_VolumeBasedMagneticFieldESProducer/'VolumeBasedMagneticFieldESProducer'
[4] Prefetching for EventSetup module DDCompactViewMFESProducer/''
[5] Calling method for EventSetup module DDDetectorESProducer/''
Exception Message:
A std::exception was thrown.
An exception of category 'DD4CMS' occurred.
Exception Message:
Composite material "materials:Water" or "materials:Water" not yet defined.

cvuosalo · 2021-12-18T14:57:05Z

@qliphy I will fix those unit tests. The fix should be simple.

cvuosalo · 2021-12-18T20:51:24Z

@qliphy #36549 should fix the unit tests listed above.

[DD4hep] Follow-up to #36496 -- Update versions of materials files in unit tests

cvuosalo added 2 commits December 11, 2021 02:25

Add exception for undefined materials

a06c806

Correct material ordering

56a0f25

cmsbuild added this to the CMSSW_12_3_X milestone Dec 14, 2021

cmsbuild added code-checks-pending geometry-pending orp-pending pending-signatures tests-pending upgrade-pending labels Dec 14, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Dec 14, 2021

cmsbuild added tests-started and removed tests-pending labels Dec 14, 2021

makortel reviewed Dec 14, 2021

View reviewed changes

cmsbuild added tests-rejected and removed tests-started labels Dec 14, 2021

Update units tests to use correctly ordered materials

0551af9

cmsbuild added code-checks-pending tests-pending and removed tests-rejected code-checks-approved labels Dec 15, 2021

cmsbuild added code-checks-rejected and removed code-checks-pending labels Dec 15, 2021

Format fix

441da8b

cmsbuild added code-checks-approved and removed code-checks-pending labels Dec 15, 2021

cvuosalo commented Dec 15, 2021

View reviewed changes

cmsbuild added tests-started and removed tests-pending labels Dec 15, 2021

cmsbuild added tests-approved and removed tests-started labels Dec 16, 2021

cmsbuild added geometry-approved and removed geometry-pending labels Dec 16, 2021

cmsbuild added fully-signed upgrade-approved and removed pending-signatures upgrade-pending labels Dec 17, 2021

cmsbuild added orp-approved and removed orp-pending labels Dec 17, 2021

cmsbuild merged commit 593eaff into cms-sw:master Dec 17, 2021

cvuosalo mentioned this pull request Dec 18, 2021

[DD4hep] Follow-up to #36496 -- Update versions of materials files in unit tests #36549

Merged

cmsbuild added a commit that referenced this pull request Dec 20, 2021

Merge pull request #36549 from cvuosalo/mat-ord-exc-pt2

0812991

[DD4hep] Follow-up to #36496 -- Update versions of materials files in unit tests

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DD4hep] Prohibit use of undefined materials #36496

[DD4hep] Prohibit use of undefined materials #36496

cvuosalo commented Dec 14, 2021 •

edited

cmsbuild commented Dec 14, 2021

cmsbuild commented Dec 14, 2021

cvuosalo commented Dec 14, 2021

civanch commented Dec 14, 2021

makortel Dec 14, 2021

cmsbuild commented Dec 14, 2021

cmsbuild commented Dec 15, 2021

cmsbuild commented Dec 15, 2021

cmsbuild commented Dec 15, 2021

cvuosalo Dec 15, 2021

cvuosalo commented Dec 15, 2021

cmsbuild commented Dec 16, 2021

civanch commented Dec 16, 2021

civanch commented Dec 17, 2021

srimanob commented Dec 17, 2021

cmsbuild commented Dec 17, 2021

perrotta commented Dec 17, 2021

qliphy commented Dec 18, 2021 •

edited

cvuosalo commented Dec 18, 2021

cvuosalo commented Dec 18, 2021

[DD4hep] Prohibit use of undefined materials #36496

[DD4hep] Prohibit use of undefined materials #36496

Conversation

cvuosalo commented Dec 14, 2021 • edited

PR validation:

cmsbuild commented Dec 14, 2021

cmsbuild commented Dec 14, 2021

cvuosalo commented Dec 14, 2021

civanch commented Dec 14, 2021

makortel Dec 14, 2021

Choose a reason for hiding this comment

cmsbuild commented Dec 14, 2021

Unit Tests

Comparison Summary

cmsbuild commented Dec 15, 2021

cmsbuild commented Dec 15, 2021

cmsbuild commented Dec 15, 2021

cvuosalo Dec 15, 2021

Choose a reason for hiding this comment

cvuosalo commented Dec 15, 2021

cmsbuild commented Dec 16, 2021

Comparison Summary

civanch commented Dec 16, 2021

civanch commented Dec 17, 2021

srimanob commented Dec 17, 2021

cmsbuild commented Dec 17, 2021

perrotta commented Dec 17, 2021

qliphy commented Dec 18, 2021 • edited

cvuosalo commented Dec 18, 2021

cvuosalo commented Dec 18, 2021

cvuosalo commented Dec 14, 2021 •

edited

qliphy commented Dec 18, 2021 •

edited