Improve DD4hep workflow perf, step 2: Remove regex from Phase 1 trackerRecoMaterial.xml #32511

ghugo83 · 2020-12-16T18:05:54Z

This also has big impact on perf in the DD4hep workflow, and no diff in perf in the DDD workflow.

NB 1: I have directly modified Geometry/TrackerRecoData/data/PhaseI/trackerRecoMaterial.xml, which is the only trackerRecoMaterial.xml for Phase 1.
If this file should be rather duplicated to be changed, please let me know and I will just move it.

NB 2: To avoid merge conflicts, this branch is on top of #32505 , which should be merged first.

@ianna @civanch @cvuosalo

…lysis/data/trackingMaterialGroups_ForPhaseI.xml, which had identical names as the SpecPar blocks in trackerRecoMaterial.xml (!!). This file was added few months ago to all geometry scenarios (why is that? as it is only used for testing materials anyway). It has a very big impact on DD4hep Run3 workflow perf. The underlying issue is that in DetectorDescription/DDCMS, when processing XMLs files for DD4hep, all SpecPar sections with same name are merged together. This results in lookups in (extremely heavy !) paths of trackingMaterialGroups_ForPhaseI.xml being made while searching for TrackerRadLength, despite TrackerRadLength not being defined in those files anyway!

…d4hep (why is there a duplicated file??)

…erial.xml. I presume the pixfwdblade.*Zplus had been done to distinguish from the pixfwdblade[0-9]:PixelForwardBlade. But this is not recessary, because the pixfwdblade[0-9]:PixelForwardBlade are not called in any geo scenario where data/PhaseI/trackerRecoMaterial.xml is used.

…these paths are not looped over when not needed anyway, hence this commit has no effect on workflow perf).

… has very small, but non-null, impact on perf.

…erial.xml. Very significant perf improvement. This version is compatible to work on both DDD and dd4hep cases.

…ngMaterialGroups_ForPhaseI.xml, version working for both DDD and dd4hep.

cmsbuild · 2020-12-16T18:13:37Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-32511/20450

This PR adds an extra 40KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File SimTracker/TrackerMaterialAnalysis/data/dd4hep_trackingMaterialGroups_ForPhaseII.xml modified in PR(s): Improve DD4hep workflow perf, step 1: All paths from SpecPar blocks with same name are stacked together #32505
- File SimTracker/TrackerMaterialAnalysis/data/trackingMaterialGroups_ForPhaseI.xml modified in PR(s): Improve DD4hep workflow perf, step 1: All paths from SpecPar blocks with same name are stacked together #32505
- File SimTracker/TrackerMaterialAnalysis/data/trackingMaterialGroups_ForPhaseII.xml modified in PR(s): Improve DD4hep workflow perf, step 1: All paths from SpecPar blocks with same name are stacked together #32505

cmsbuild · 2020-12-16T18:14:05Z

A new Pull Request was created by @ghugo83 for master.

It involves the following packages:

Geometry/TrackerRecoData
SimTracker/TrackerMaterialAnalysis

@civanch, @Dr15Jones, @makortel, @cvuosalo, @ianna, @mdhildreth, @cmsbuild can you please review it and eventually sign? Thanks.
@fabiocos, @vargasa, @makortel, @mtosi, @GiacomoSguazzoni, @JanFSchulte, @rovere, @VinInn, @apsallid, @ebrondol, @threus, @dgulhan this is something you requested to watch as well.
@silviodonato, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

cvuosalo · 2020-12-16T18:51:41Z

Phase I (Run 3) XML files must be given a new version number when changed.

ghugo83 · 2020-12-16T19:00:25Z

Phase I (Run 3) XML files must be given a new version number when changed.

Ok, will duplicate that file then.

…try scenarios.

cmsbuild · 2020-12-17T11:29:38Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-32511/20464

This PR adds an extra 68KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File SimTracker/TrackerMaterialAnalysis/data/dd4hep_trackingMaterialGroups_ForPhaseII.xml modified in PR(s): Improve DD4hep workflow perf, step 1: All paths from SpecPar blocks with same name are stacked together #32505
- File SimTracker/TrackerMaterialAnalysis/data/trackingMaterialGroups_ForPhaseI.xml modified in PR(s): Improve DD4hep workflow perf, step 1: All paths from SpecPar blocks with same name are stacked together #32505
- File SimTracker/TrackerMaterialAnalysis/data/trackingMaterialGroups_ForPhaseII.xml modified in PR(s): Improve DD4hep workflow perf, step 1: All paths from SpecPar blocks with same name are stacked together #32505

cvuosalo · 2020-12-18T17:11:38Z

@cmsbuild please test

cmsbuild · 2020-12-18T22:09:53Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-258ecf/11800/summary.html
CMSSW: CMSSW_11_3_X_2020-12-18-1100/slc7_amd64_gcc900

DAS Queries failed

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 7 differences found in the comparisons
DQMHistoTests: Total files compared: 36
DQMHistoTests: Total histograms compared: 2716967
DQMHistoTests: Total failures: 12
DQMHistoTests: Total nulls: 1
DQMHistoTests: Total successes: 2716932
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.004 KiB( 35 files compared)
DQMHistoSizes: changed ( 312.0 ): 0.004 KiB MessageLogger/Warnings
Checked 153 log files, 37 edm output root files, 36 DQM output files

cvuosalo · 2020-12-18T22:59:51Z

@ghugo83 The name for the trackingMaterialGroups_ForPhaseI file needs to be SimTracker/TrackerMaterialAnalysis/data/trackingMaterialGroups_ForPhaseI/v1/trackingMaterialGroups_ForPhaseI.xml.

civanch · 2020-12-19T18:36:17Z

@cvuosalo , are you sure about this SimTracker/TrackerMaterialAnalysis/data/? Is it a part of DB payload?

civanch · 2020-12-20T16:51:10Z

+1

Let us merge this PR and if extra updates are needed implement in the following PR, because we should be able to see effect of these updates. @kpedro88 , please, have a look.

kpedro88 · 2020-12-20T17:06:13Z

+upgrade

cmsbuild · 2020-12-20T17:06:38Z

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, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

qliphy · 2020-12-21T00:07:26Z

+1

silviodonato · 2020-12-21T08:56:25Z

@ghugo83 there is a conflict

mrodozov · 2020-12-21T10:25:19Z

@silviodonato I'm following, it looks merging went fine . If something is breaking we can revert to rebase and restart the build

ghugo83 · 2020-12-21T14:22:01Z

Thanks, sorry was travelling.
Just had a quick look and evth is ok.

I will just rename SimTracker/TrackerMaterialAnalysis/data/v1/trackingMaterialGroups_ForPhaseI.xml to SimTracker/TrackerMaterialAnalysis/data/trackingMaterialGroups_ForPhaseI/v1/trackingMaterialGroups_ForPhaseI.xml as suggested, but that is just a renaming.
I will do it quickly tonight when I am back to a computer ;p
Thanks again,
Gabrielle

ghugo83 · 2020-12-21T14:24:46Z

Merge conflict: as mentioned there should be none. This branch is purposefully based on top of the 'step 1' one, so that there should be no conflict to fix to merge this PR, once step 1 is merged :)
This PR properly restore SimTracker/TrackerMaterialAnalysis/data/trackingMaterialGroups_ForPhaseI.xml and use a dedicated v1 version instead.
So evth went as expected :)

cvuosalo · 2020-12-21T22:06:28Z

@civanch trackingMaterialGroups_ForPhaseI.xml is in the 2021 dictionary for scenario T3, which is used for the 2021 extended geometry, so I think the previous version of this file was included in the current Run 3 geometry in the DB.
I will fix the names in a new PR.

ghugo83 added 8 commits December 16, 2020 13:04

Also remane SpecPar sections in trackingMaterialGroups for PhaseII

8b44619

Also remane SpecPar sections in trackingMaterialGroups for PhaseII, d…

6d46900

…d4hep (why is there a duplicated file??)

Also remove regex from trackingMaterialGroups_ForPhaseI.xml (but now …

f52b55b

…these paths are not looped over when not needed anyway, hence this commit has no effect on workflow perf).

Truncate the unnecessary Tracker/subdetector heads in the paths. This…

cc83df3

… has very small, but non-null, impact on perf.

Remove regex from Geometry/TrackerRecoData/data/PhaseI/trackerRecoMat…

51565c8

…erial.xml. Very significant perf improvement. This version is compatible to work on both DDD and dd4hep cases.

Also remove regex from SimTracker/TrackerMaterialAnalysis/data/tracki…

aaf4029

…ngMaterialGroups_ForPhaseI.xml, version working for both DDD and dd4hep.

cmsbuild added this to the CMSSW_11_3_X milestone Dec 16, 2020

cmsbuild added code-checks-pending geometry-pending orp-pending pending-signatures simulation-pending tests-pending labels Dec 16, 2020

cmsbuild added code-checks-approved and removed code-checks-pending labels Dec 16, 2020

ghugo83 added 3 commits December 17, 2020 11:47

Create v1 for Phase 1 trackerRecoMaterial.xml

0880035

Call no-regex version of Phase1/trackerRecoMaterial.xml in 2021 geome…

95053db

…try scenarios.

Also load the no-regex XML file in materials test.

0ff94d6

cmsbuild added code-checks-pending upgrade-pending and removed code-checks-approved labels Dec 17, 2020

cmsbuild added code-checks-approved and removed code-checks-pending labels Dec 17, 2020

ghugo83 mentioned this pull request Dec 18, 2020

Improve DD4hep workflow perf, step 4: Improve performance of GeometricDet construction in old DD (DDD) and DD4hep #32544

Merged

cmsbuild added tests-started and removed tests-pending labels Dec 18, 2020

ghugo83 mentioned this pull request Dec 18, 2020

Improve DD4hep workflow perf, step 1: All paths from SpecPar blocks with same name are stacked together #32505

Merged

cmsbuild added tests-approved and removed tests-started labels Dec 18, 2020

cmsbuild added geometry-approved simulation-approved and removed geometry-pending simulation-pending labels Dec 20, 2020

cmsbuild added fully-signed upgrade-approved and removed pending-signatures upgrade-pending labels Dec 20, 2020

cmsbuild added orp-approved and removed orp-pending labels Dec 21, 2020

cmsbuild merged commit da19975 into cms-sw:master Dec 21, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve DD4hep workflow perf, step 2: Remove regex from Phase 1 trackerRecoMaterial.xml #32511

Improve DD4hep workflow perf, step 2: Remove regex from Phase 1 trackerRecoMaterial.xml #32511

ghugo83 commented Dec 16, 2020 •

edited

cmsbuild commented Dec 16, 2020

cmsbuild commented Dec 16, 2020

cvuosalo commented Dec 16, 2020

ghugo83 commented Dec 16, 2020

cmsbuild commented Dec 17, 2020

cvuosalo commented Dec 18, 2020

cmsbuild commented Dec 18, 2020

cvuosalo commented Dec 18, 2020

civanch commented Dec 19, 2020

civanch commented Dec 20, 2020

kpedro88 commented Dec 20, 2020

cmsbuild commented Dec 20, 2020

qliphy commented Dec 21, 2020

silviodonato commented Dec 21, 2020

mrodozov commented Dec 21, 2020

ghugo83 commented Dec 21, 2020 •

edited

ghugo83 commented Dec 21, 2020 •

edited

cvuosalo commented Dec 21, 2020

Improve DD4hep workflow perf, step 2: Remove regex from Phase 1 trackerRecoMaterial.xml #32511

Improve DD4hep workflow perf, step 2: Remove regex from Phase 1 trackerRecoMaterial.xml #32511

Conversation

ghugo83 commented Dec 16, 2020 • edited

cmsbuild commented Dec 16, 2020

cmsbuild commented Dec 16, 2020

cvuosalo commented Dec 16, 2020

ghugo83 commented Dec 16, 2020

cmsbuild commented Dec 17, 2020

cvuosalo commented Dec 18, 2020

cmsbuild commented Dec 18, 2020

Comparison Summary

cvuosalo commented Dec 18, 2020

civanch commented Dec 19, 2020

civanch commented Dec 20, 2020

kpedro88 commented Dec 20, 2020

cmsbuild commented Dec 20, 2020

qliphy commented Dec 21, 2020

silviodonato commented Dec 21, 2020

mrodozov commented Dec 21, 2020

ghugo83 commented Dec 21, 2020 • edited

ghugo83 commented Dec 21, 2020 • edited

cvuosalo commented Dec 21, 2020

ghugo83 commented Dec 16, 2020 •

edited

ghugo83 commented Dec 21, 2020 •

edited

ghugo83 commented Dec 21, 2020 •

edited