Adding geometry scenarios D59 and D60 implementing ETL version 4 and 5 #30340

icosivi · 2020-06-23T12:01:56Z

PR description:

This PR implements geometry scenarios D59 and D60:

D59 modifies scenario D58 introducing ETL version 4
D60 modifies scenario D58 introducing ETL version 5

ETL version 4 was already implemented in scenario D53 (https://indico.cern.ch/event/868736/contributions/3709315/attachments/1978993/3295064/MTDdays_20200131.pdf) and fits in the new Endcap Calorimeter envelope defined in D58.
ETL version 5 implements the latest version of ETL disks, as proposed by N. Koss (https://indico.cern.ch/event/868736/contributions/3706894/attachments/1978848/3294508/20200130_ETL_mechanics_timing_days.pdf).

ETL v5 introduces D-shaped sectors, instead of the quarters used in ETL v4, comprising more than 512 modules, which is the maximum number of modules that could be accommodated using the allocated bits in ETLDetId. Therefore ETLDetId.h has been modified to fix this issue, by adding 2 bits. The new ETLDetId is backward compatible with the older versions, thanks to a dedicated compatibility code implemented in ETLDetId.cc.

Both ETL versions and the backward compatibility issue were presented at the last MTD DPG meeting (https://indico.cern.ch/event/919571/contributions/3864046/attachments/2039610/3415525/MTD_DPG_20200515.pdf )

PR validation:

Both scenarios have been tested using testMTDinDDD.py and testMTDinDD4hep.py: the dump geometries have been checked and both volumes positions and logical paths are correct.
The overlaps have been checked too using g4OverlapCheck_cfg.py.
The dump geometries for scenario D50 using the old ETLDetId format and the new one described above have been compared: the logical volumes paths are the same, while the rawID differ only for the positions of the 2 bits related to the sensor modules, as expected. The new ETLDetId format is therefore correct and backward compatible.

… offset)

…lity

…ests ok.

cmsbuild · 2020-06-23T12:02:18Z

The code-checks are being triggered in jenkins.

cmsbuild · 2020-06-23T12:11:07Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-30340/16351

This PR adds an extra 96KB to repository
Found files with invalid states:
- Geometry/MTDCommonData/data/dd4hep/cms-mtdD57-geometry.xml:
  - Added: df1e818
  - Deleted: 593ac79
There are other open Pull requests which might conflict with changes you have proposed:
- File Configuration/Geometry/python/dict2026Geometry.py modified in PR(s): Run4-hgx248 Add ERA corresponding to V12 Geometry #30305, update the tracking material for Phase2 geometry D49 #30126

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

cmsbuild · 2020-06-23T13:16:44Z

The code-checks are being triggered in jenkins.

cmsbuild · 2020-06-23T13:22:52Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-30340/16355

This PR adds an extra 96KB to repository
Found files with invalid states:
- Geometry/MTDCommonData/data/dd4hep/cms-mtdD57-geometry.xml:
  - Added: df1e818
  - Deleted: 593ac79
There are other open Pull requests which might conflict with changes you have proposed:
- File Configuration/Geometry/python/dict2026Geometry.py modified in PR(s): Run4-hgx248 Add ERA corresponding to V12 Geometry #30305, update the tracking material for Phase2 geometry D49 #30126

cmsbuild · 2020-06-25T08:31:53Z

-1

Tested at: 0e905af

CMSSW: CMSSW_11_2_X_2020-06-24-2300
SCRAM_ARCH: slc7_amd64_gcc820
You can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-9f212c/7334/summary.html

I found follow errors while testing this PR

Failed tests: UnitTests RelVals

Unit Tests:

I found errors in the following unit tests:

---> test GeometryMTDGeometryBuilderTestDriver had ERRORS
---> test GeometryMTDNumberingBuilderTestDriver had ERRORS
---> test GeometryMTDCommonDataTestDriver had ERRORS

RelVals:

When I ran the RelVals I found an error in the following workflows:
23234.1001 step1

runTheMatrix-results/23234.1001_TTbar_14TeV+RecoFullGlobal_TestOldDigi_2026D49+HARVESTFullGlobal_2026D49/step1_TTbar_14TeV+RecoFullGlobal_TestOldDigi_2026D49+HARVESTFullGlobal_2026D49.log

20034.0 step3

runTheMatrix-results/20034.0_TTbar_14TeV+TTbar_14TeV_TuneCP5_2026D35_GenSimHLBeamSpotFull14+DigiFullTrigger_2026D35+RecoFullGlobal_2026D35+HARVESTFullGlobal_2026D35/step3_TTbar_14TeV+TTbar_14TeV_TuneCP5_2026D35_GenSimHLBeamSpotFull14+DigiFullTrigger_2026D35+RecoFullGlobal_2026D35+HARVESTFullGlobal_2026D35.log

20434.0 step3

runTheMatrix-results/20434.0_TTbar_14TeV+TTbar_14TeV_TuneCP5_2026D41_GenSimHLBeamSpotFull14+DigiFullTrigger_2026D41+RecoFullGlobal_2026D41+HARVESTFullGlobal_2026D41/step3_TTbar_14TeV+TTbar_14TeV_TuneCP5_2026D41_GenSimHLBeamSpotFull14+DigiFullTrigger_2026D41+RecoFullGlobal_2026D41+HARVESTFullGlobal_2026D41.log

23234.0 step3

runTheMatrix-results/23234.0_TTbar_14TeV+TTbar_14TeV_TuneCP5_2026D49_GenSimHLBeamSpotFull14+DigiFullTrigger_2026D49+RecoFullGlobal_2026D49+HARVESTFullGlobal_2026D49/step3_TTbar_14TeV+TTbar_14TeV_TuneCP5_2026D49_GenSimHLBeamSpotFull14+DigiFullTrigger_2026D49+RecoFullGlobal_2026D49+HARVESTFullGlobal_2026D49.log

21234.0 step3

runTheMatrix-results/21234.0_TTbar_14TeV+TTbar_14TeV_TuneCP5_2026D44_GenSimHLBeamSpotFull14+DigiFullTrigger_2026D44+RecoFullGlobal_2026D44+HARVESTFullGlobal_2026D44/step3_TTbar_14TeV+TTbar_14TeV_TuneCP5_2026D44_GenSimHLBeamSpotFull14+DigiFullTrigger_2026D44+RecoFullGlobal_2026D44+HARVESTFullGlobal_2026D44.log

cmsbuild · 2020-06-25T08:31:55Z

Comparison not run due to runTheMatrix errors (RelVals and Igprof tests were also skipped)

cmsbuild · 2020-06-26T17:11:14Z

The code-checks are being triggered in jenkins.

kpedro88 · 2020-06-26T17:29:44Z

@icosivi does the latest commit address the test failure? if not, please make an explicit comment when the PR is expected to work. (As always, please test the PR locally first.)

cmsbuild · 2020-06-26T18:29:50Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-30340/16534

This PR adds an extra 28KB to repository
Found files with invalid states:
- Geometry/MTDCommonData/data/dd4hep/cms-mtdD57-geometry.xml:
  - Added: df1e818
  - Deleted: 593ac79
There are other open Pull requests which might conflict with changes you have proposed:
- File Configuration/Geometry/python/dict2026Geometry.py modified in PR(s): update the tracking material for Phase2 geometry D49 #30126

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

icosivi · 2020-06-26T18:51:09Z

@kpedro88 not yet, I will make an explicit comment when it will be expected to work, thanks

fabiocos · 2020-06-27T16:26:23Z

@kpedro88 in my understanding from @icosivi this PR was locally tested with the standalone geometry checks, and no obvious issue was found. The failure of the unit test in itself is totally trivial, as ETL DetIds change a new reference needs to be used. What is clearly unexpected is the failure in the old relvals, which is of course something that needs to be verified to work. The test of reading old files with new code and verify that the translation was properly done was done (@icosivi please confirm). But apparently there is some unexpected issue in the chain. The Geant4 step looks ok from the tests I have suggested, while a closer check to DIGIs is needed, and the update of the dumper I asked @icosivi is meant for this.

I believe this PR could be put at least on hold. But probably we could have a different strategy: split it into a PR to update ETLDetId, once all checks are completed and problems identified and solved, and another one with the remaining update of new scenarios, that looks less prone to create issues.

icosivi · 2020-06-29T08:33:43Z

@fabiocos I confirm that the test of reading old files with new code was done

fabiocos · 2020-06-30T13:43:42Z

hold

this may be eventually split into a PR for ETLDetId, and another one with the new scenarios (updated for the new work by @bsunanda )

fabiocos · 2020-06-30T14:34:43Z

DataFormats/ForwardDetId/src/classes_def.xml

   <version ClassVersion="3" checksum="1644731879"/>
+   <ioread sourceClass = "ETLDetId" version="[-4]" targetClass="ETLDetId" source="" target="">


@icosivi @casarsa most likely the version here should be [-3], we want the conversion on older ClassVersion data. This would explain why everything looks ok while writing, but then it miserably fails after reading back data from file.

while it looks ok reading data from previous versions...

icosivi added 9 commits June 18, 2020 17:35

1st commit. Scenario D57 implementing etl/v5 added

df1e818

v3/mtdStructureTopology.xml added. MTDTopologyMode modified.

17c1d63

ETLDetId modified. 2 bits added to module and modType (both mask and…

2675162

… offset)

Modified EtlDetId.h

707b812

Fixed DetId for etl v5

a7f13e1

Added scenario D59 (ETL v5). ETLDetId modified for backward compatibi…

7f2a9a3

…lity

adding cms-mtdD59-geometry.xml

593ac79

Added scenarios D59 (D58+etl_v4) and D60 (D58+etl_v5). Overlap and t…

dbab7a5

…ests ok.

Fixed constants in v4/etl.xml and v5/etl.xml

8bdcb36

cmsbuild added this to the CMSSW_11_2_X milestone Jun 23, 2020

cmsbuild added code-checks-pending comparison-pending geometry-pending operations-pending orp-pending pending-signatures simulation-pending tests-pending upgrade-pending labels Jun 23, 2020

cmsbuild added code-checks-rejected and removed code-checks-pending labels Jun 23, 2020

Code-format applied

0e905af

cmsbuild added code-checks-pending and removed code-checks-rejected labels Jun 23, 2020

cmsbuild added code-checks-approved and removed code-checks-pending labels Jun 23, 2020

cmsbuild added tests-started and removed tests-pending labels Jun 25, 2020

cmsbuild added comparison-notrun tests-rejected and removed comparison-pending tests-started labels Jun 25, 2020

SimDigiDumper modified, adding ETL dump

54e8e45

cmsbuild added code-checks-pending comparison-pending tests-pending and removed code-checks-approved comparison-notrun tests-rejected labels Jun 26, 2020

cmsbuild added code-checks-rejected and removed code-checks-pending labels Jun 26, 2020

fabiocos mentioned this pull request Jun 28, 2020

SimGeneral/Debugging: add MTD to the digi dump #30441

Merged

cmsbuild mentioned this pull request Jun 28, 2020

Phase2-geox02 Make workflow for a version with HGCal V13 #30442

Merged

cmsbuild added the hold label Jun 30, 2020

fabiocos reviewed Jun 30, 2020

View reviewed changes

cmsbuild mentioned this pull request Jul 1, 2020

Unify geometry scripts #30477

Merged

icosivi closed this Jul 1, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding geometry scenarios D59 and D60 implementing ETL version 4 and 5 #30340

Adding geometry scenarios D59 and D60 implementing ETL version 4 and 5 #30340

icosivi commented Jun 23, 2020

cmsbuild commented Jun 23, 2020

cmsbuild commented Jun 23, 2020

cmsbuild commented Jun 23, 2020

cmsbuild commented Jun 23, 2020

cmsbuild commented Jun 25, 2020

cmsbuild commented Jun 25, 2020

cmsbuild commented Jun 26, 2020

kpedro88 commented Jun 26, 2020

cmsbuild commented Jun 26, 2020

icosivi commented Jun 26, 2020

fabiocos commented Jun 27, 2020

icosivi commented Jun 29, 2020

fabiocos commented Jun 30, 2020

fabiocos Jun 30, 2020

fabiocos Jun 30, 2020

		<version ClassVersion="3" checksum="1644731879"/>
		<ioread sourceClass = "ETLDetId" version="[-4]" targetClass="ETLDetId" source="" target="">

Adding geometry scenarios D59 and D60 implementing ETL version 4 and 5 #30340

Adding geometry scenarios D59 and D60 implementing ETL version 4 and 5 #30340

Conversation

icosivi commented Jun 23, 2020

PR description:

PR validation:

cmsbuild commented Jun 23, 2020

cmsbuild commented Jun 23, 2020

cmsbuild commented Jun 23, 2020

cmsbuild commented Jun 23, 2020

cmsbuild commented Jun 25, 2020

cmsbuild commented Jun 25, 2020

cmsbuild commented Jun 26, 2020

kpedro88 commented Jun 26, 2020

cmsbuild commented Jun 26, 2020

icosivi commented Jun 26, 2020

fabiocos commented Jun 27, 2020

icosivi commented Jun 29, 2020

fabiocos commented Jun 30, 2020

fabiocos Jun 30, 2020

Choose a reason for hiding this comment

fabiocos Jun 30, 2020

Choose a reason for hiding this comment