[DD4hep] Allow Use of Namespace in SpecPar Comparison #31579

ianna · 2020-09-25T08:54:28Z

requested by @ghugo83 (see DD4hep: Introduces functional Tracker navigation + DetIds building #31573)

cmsbuild · 2020-09-25T08:54:48Z

A new Issue was created by @ianna Ianna Osborne.

@Dr15Jones, @dpiparo, @silviodonato, @smuzaffar, @makortel, @qliphy can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

ianna · 2020-09-25T08:57:05Z

assign geometry
urgent

cmsbuild · 2020-09-25T08:57:23Z

New categories assigned: geometry

@Dr15Jones,@cvuosalo,@mdhildreth,@makortel,@ianna,@civanch you have been requested to review this Pull request/Issue and eventually sign? Thanks

ianna · 2020-09-25T09:33:41Z

urgent

cvuosalo · 2020-09-25T22:23:53Z

Who is working on this issue?

cvuosalo · 2020-09-28T12:56:50Z

@ghugo83 How would you suggest the namespace be handled in the Filtered View with respect to the SpecPars? Sometimes we want to namespace excluded, and sometimes we want it included. How should that be specified?

ianna · 2020-09-28T13:05:42Z

@ghugo83 How would you suggest the namespace be handled in the Filtered View with respect to the SpecPars? Sometimes we want to namespace excluded, and sometimes we want it included. How should that be specified?

I'm about to submit a PR to provide an additional getter from the spec pars that does not strip off the namespace. It would mean that all the volumes should be crated with the namespace of interest. That needs to be checked, also if there is an extra cost of it.

ghugo83 · 2020-09-28T15:59:00Z

Yep I think the behavior of the DD4hep Filtered view interface should be the same as old DD:

If no namespace is specified (or use of .*: ) in XML SpecPars: then all volumes matching the regex are selected.
Example:
<PartSelector path="//.*:PixelSensor" /> or <PartSelector path="//PixelSensor" />
Here, in old DD, both tdr:PixelSensor or mytest:PixelSensor would be selected. It should be the same for DD4hep.
If a namespace is specified: then I think we always want to select the nodes matching that namespace only.
Example:
<PartSelector path="//mytest:PixelSensor" />
Here, we want all nodes containing mytest:PixelSensor to be selected, but we do not want tdr:PixelSensor to be selected (neither in old DD nor in DD4hep).

ghugo83 · 2020-09-28T16:11:00Z

It should be the same for copy number: when copy number is explicitly specified in the SpecPars in XMLs, it should be taken into account, and if not, it should not.

ianna · 2020-09-28T16:15:17Z

It should be the same for copy number: when copy number is explicitly specified in the SpecPars in XMLs, it should be taken into account, and if not, it should not.

yes, that's there already :-)

ghugo83 · 2020-09-28T16:17:03Z

In reco XMLs for example, there are paths like this:
<PartSelector path="//PixelBarrel/PixelBarrelLayer0/PixelBarrelLadderFull0/pixbarladderfull0:PixelBarrelModuleBoxFull/pixbarladderfull0:PixelBarrelModuleFullPlus[1]/pixbarladderfull0:PixelBarrelSensorFull/PixelBarrelActiveFull0" />
Which should work for both old DD and DD4hep.

A DD4hep path is:
world_volume/OCMS_1/CMSE_1/Tracker_1/PixelBarrel_1/pixbarlayer0:PixelBarrelLayer0_1/PixelBarrelLadderFull0_1/PixelBarrelModuleBoxFull_1/PixelBarrelModuleFullPlus_1/PixelBarrelSensorFull_1/PixelBarrelActiveFull0_1
I think one way is to have the DD4hep path converted and stored as an 'old DD expression'.
Namespace should be forced to be saved (we know in which file volumes are defined anyway). And then the namespace is only taken into account if it does appear in the XML SpecPar.

ianna · 2020-09-29T06:53:37Z

In reco XMLs for example, there are paths like this:
<PartSelector path="//PixelBarrel/PixelBarrelLayer0/PixelBarrelLadderFull0/pixbarladderfull0:PixelBarrelModuleBoxFull/pixbarladderfull0:PixelBarrelModuleFullPlus[1]/pixbarladderfull0:PixelBarrelSensorFull/PixelBarrelActiveFull0" />
Which should work for both old DD and DD4hep.

A DD4hep path is:
world_volume/OCMS_1/CMSE_1/Tracker_1/PixelBarrel_1/pixbarlayer0:PixelBarrelLayer0_1/PixelBarrelLadderFull0_1/PixelBarrelModuleBoxFull_1/PixelBarrelModuleFullPlus_1/PixelBarrelSensorFull_1/PixelBarrelActiveFull0_1
I think one way is to have the DD4hep path converted and stored as an 'old DD expression'.

@ghugo83 - this conversion may cost as much as its comparison, e.g. what is already done now (but without a namespaces). Namespace in DD4hep is a Volume name part that is one std::string, it's separated by ":".
To first extend, changing it to a "!" for example, will allow a full name comparison. Though, it will not work as it will require the PartSelector paths include all namespaces...

Namespace should be forced to be saved (we know in which file volumes are defined anyway). And then the namespace is only taken into account if it does appear in the XML SpecPar.

It's an extra check per Volume per path :-)

SpecPars are not attached to the Volumes, but exist in a separate registry. That is why I was talking about using the TGeo plugins eventually. These are executed on visiting the Node.

Anyway, I have an interim solution in my local area that is not too costly and will PR it soon. :-)

ghugo83 · 2020-09-29T08:21:00Z

@ianna Great! Yes am not working on this front anyway (exploring the others points of the to-do list), I trust you for finding sth which works with an acceptable perf :)

ghugo83 · 2020-09-29T08:22:04Z

By the way I think I have found a way to fasten the DD4hep Tracker workflow (off-topic from this issue though).

ianna · 2020-09-29T09:03:48Z

@ghugo83 - I'll PR on this issue after the #31610 is merged. This will help you to integrate #31573 faster because "test with" bundles the PRs together. At least we'll be done with a step 1 :-)

ianna · 2020-09-29T15:36:34Z

@ghugo83 and @vargasa - I think, I'm done with it. I have a unit test that accesses one module values. They seem correct.
Is there full Tracker test to check rad length and xi? Thanks

ianna · 2020-09-29T15:49:30Z

fixed in #31614

ianna · 2020-09-29T15:49:46Z

+1

cmsbuild · 2020-09-29T15:50:03Z

This issue is fully signed and ready to be closed.

ghugo83 · 2020-09-29T16:08:49Z

@ianna
I think a printout, in the constructor body, of:

cmssw/Geometry/TrackerNumberingBuilder/src/GeometricDet.cc

Line 109 in 41af5ee

radLength_(fv->get<double>("TrackerRadLength")), // To be studied

(on top of my PR) is the most realistic test, because it is the value which gets stored in GeometricDet.

Comparison:
(a) just merging my PR
VERSUS
(b) merging my PR + your fix + removing the XML with no namespace (Geometry/TrackerCommonData/data/PhaseI/dd4hep/trackerStructureTopology.xml), and using old DD XML instead (Geometry/TrackerCommonData/data/PhaseI/trackerStructureTopology.xml).
Personally I got issues with (b) but I am trying to re-check on another release.

ghugo83 · 2020-09-29T16:18:29Z

By the way, 90% of all the time used to create the full Tracker geometry and numbering, is used by the calls of (already there before my PR):
DDFilteredView::get<type>
in the GeometricDet constructor.

If you comment these 10 lines out, you get a x10 speedup with DD4hep.
More specifically, the 2 lines trying to get Radiation Length and EnergyLoss from the XMLs take most of the time. I guess that's because the SpecPars in these XMLs are very long (not one volume name only).

DDFilteredView::firstChild() is not that bad (anyway it is within a (small?) fraction of the remaining 10 %).
I had doubts about fv->firstChild() being that bad, because I also used it for COCOA and PPS, and did not find any significant slowdown.
Though, it seems to mark a big wait when the last volume of the filtered view is reached, before it displays 'no more first child found'. Nothing comparable to DDFilteredView::get<type> though.

ghugo83 · 2020-09-29T16:45:41Z

Hmm yes the fix in (b) (no-namespace) does not seem to work yet.
<PartSelector path="//.*:PixelForwardWafer" />
The panel is not recognized, hence the disks have no panel nor sensor.
The disks instead are successfully created, because they are defined differently, with <PartSelector path="//PixelForwardDiskZplus" /> in the XML (no regex).

ianna · 2020-09-30T07:41:18Z

@ghugo83 - thanks, now that the PRs are merged I should try to reproduce it with your branch.

ianna · 2020-09-30T08:22:09Z

Hmm yes the fix in (b) (no-namespace) does not seem to work yet.
<PartSelector path="//.*:PixelForwardWafer" />
The panel is not recognized, hence the disks have no panel nor sensor.
The disks instead are successfully created, because they are defined differently, with <PartSelector path="//PixelForwardDiskZplus" /> in the XML (no regex).

@ghugo83 - fixed in #31621

ianna · 2020-09-30T08:31:19Z

By the way, 90% of all the time used to create the full Tracker geometry and numbering, is used by the calls of (already there before my PR):
DDFilteredView::get<type>
in the GeometricDet constructor.

If you comment these 10 lines out, you get a x10 speedup with DD4hep.
More specifically, the 2 lines trying to get Radiation Length and EnergyLoss from the XMLs take most of the time. I guess that's because the SpecPars in these XMLs are very long (not one volume name only).

DDFilteredView::firstChild() is not that bad (anyway it is within a (small?) fraction of the remaining 10 %).
I had doubts about fv->firstChild() being that bad, because I also used it for COCOA and PPS, and did not find any significant slowdown.
Though, it seems to mark a big wait when the last volume of the filtered view is reached, before it displays 'no more first child found'. Nothing comparable to DDFilteredView::get<type> though.

@ghugo83 - this PR #31614 should address some performance issues. Thought it may still be a premature optimisation ( "the root of all evil (or at least most of it)" ) :-)

ghugo83 · 2020-09-30T12:02:31Z

Hmm yes the fix in (b) (no-namespace) does not seem to work yet.

The panel is not recognized, hence the disks have no panel nor sensor.
The disks instead are successfully created, because they are defined differently, with path="//PixelForwardDiskZplus" /> in the XML (no regex).

@ghugo83 - fixed in #31621

Ok will re-test that

ianna · 2020-09-30T12:32:16Z

@ghugo83 - FYI testing your PR #31573 + PR #31621 with the following change shows no differences:

$ git diff
diff --git a/Geometry/CMSCommonData/data/dd4hep/cmsExtendedGeometry2021.xml b/Geometry/CMSCommonData/data/dd4hep/cmsExtendedGeometry2021.xml
index 92ca1793acf..6ce3f568fec 100644
--- a/Geometry/CMSCommonData/data/dd4hep/cmsExtendedGeometry2021.xml
+++ b/Geometry/CMSCommonData/data/dd4hep/cmsExtendedGeometry2021.xml
@@ -206,7 +206,7 @@
     <Include ref='Geometry/TrackerCommonData/data/trackertec.xml'/>
     <Include ref='Geometry/TrackerCommonData/data/v2/trackerbulkhead.xml'/>
     <Include ref='Geometry/TrackerCommonData/data/trackerother.xml'/>
-    <Include ref='Geometry/TrackerCommonData/data/PhaseI/dd4hep/trackerStructureTopology.xml'/>
+    <Include ref='Geometry/TrackerCommonData/data/PhaseI/trackerStructureTopology.xml'/>
     <Include ref='Geometry/TrackerSimData/data/PhaseI/trackersens.xml'/>
     <Include ref='Geometry/TrackerRecoData/data/PhaseI/trackerRecoMaterial.xml'/>
     <Include ref='SimTracker/TrackerMaterialAnalysis/data/trackingMaterialGroups_ForPhaseI.xml'/>

11183c11183
< %MSG-i Root_Information:  ModuleNumbering:prod TGeoManager::CheckGeometry() 30-Sep-2020 11:22:49 CEST Run: 1 Event: 1
---
> %MSG-i Root_Information:  ModuleNumbering:prod TGeoManager::CheckGeometry() 30-Sep-2020 11:37:31 CEST Run: 1 Event: 1
11186c11186
< %MSG-i Root_Information:  ModuleNumbering:prod TGeoManager::CloseGeometry() 30-Sep-2020 11:22:49 CEST Run: 1 Event: 1
---
> %MSG-i Root_Information:  ModuleNumbering:prod TGeoManager::CloseGeometry() 30-Sep-2020 11:37:31 CEST Run: 1 Event: 1
11189c11189
< %MSG-i Root_Information:  ModuleNumbering:prod TGeoManager::Voxelize() 30-Sep-2020 11:22:49 CEST Run: 1 Event: 1
---
> %MSG-i Root_Information:  ModuleNumbering:prod TGeoManager::Voxelize() 30-Sep-2020 11:37:31 CEST Run: 1 Event: 1
11192c11192
< %MSG-i Root_Information:  ModuleNumbering:prod TGeoManager::CloseGeometry() 30-Sep-2020 11:22:49 CEST Run: 1 Event: 1
---
> %MSG-i Root_Information:  ModuleNumbering:prod TGeoManager::CloseGeometry() 30-Sep-2020 11:37:31 CEST Run: 1 Event: 1
11195c11195
< %MSG-i Root_Information:  ModuleNumbering:prod TGeoManager::CountLevels() 30-Sep-2020 11:22:50 CEST Run: 1 Event: 1
---
> %MSG-i Root_Information:  ModuleNumbering:prod TGeoManager::CountLevels() 30-Sep-2020 11:37:31 CEST Run: 1 Event: 1
11198c11198
< %MSG-i Root_Information:  ModuleNumbering:prod TGeoManager::CloseGeometry() 30-Sep-2020 11:22:50 CEST Run: 1 Event: 1
---
> %MSG-i Root_Information:  ModuleNumbering:prod TGeoManager::CloseGeometry() 30-Sep-2020 11:37:31 CEST Run: 1 Event: 1
11201c11201
< %MSG-i Root_Information:  ModuleNumbering:prod TGeoManager::CloseGeometry() 30-Sep-2020 11:22:50 CEST Run: 1 Event: 1
---
> %MSG-i Root_Information:  ModuleNumbering:prod TGeoManager::CloseGeometry() 30-Sep-2020 11:37:31 CEST Run: 1 Event: 1
11206c11206
< %MSG-i TrackerGeom:  ModuleNumbering:prod 30-Sep-2020 11:22:50 CEST Run: 1 Event: 1
---
> %MSG-i TrackerGeom:  ModuleNumbering:prod 30-Sep-2020 11:37:31 CEST Run: 1 Event: 1
11209c11209
< %MSG-i TrackerTopologyEP:  ModuleNumbering:prod 30-Sep-2020 11:22:50 CEST Run: 1 Event: 1
---
> %MSG-i TrackerTopologyEP:  ModuleNumbering:prod 30-Sep-2020 11:37:31 CEST Run: 1 Event: 1
11212c11212
< %MSG-i ModuleNumbering:  ModuleNumbering:prod 30-Sep-2020 11:22:50 CEST Run: 1 Event: 1
---
> %MSG-i ModuleNumbering:  ModuleNumbering:prod 30-Sep-2020 11:37:31 CEST Run: 1 Event: 1
55604,55605c55604,55605
< %MSG-i ModuleNumbering:  ModuleNumbering:prod 30-Sep-2020 11:34:40 CEST Run: 1 Event: 1
<  Top node is  0x7ff3a3fc34c0 Tracker
---
> %MSG-i ModuleNumbering:  ModuleNumbering:prod 30-Sep-2020 11:49:20 CEST Run: 1 Event: 1
>  Top node is  0x7fcb6ed824c0 Tracker
55607c55607
< %MSG-i ModuleNumbering:  ModuleNumbering:prod 30-Sep-2020 11:34:40 CEST Run: 1 Event: 1
---
> %MSG-i ModuleNumbering:  ModuleNumbering:prod 30-Sep-2020 11:49:20 CEST Run: 1 Event: 1
55618c55618
< %MSG-i ModuleNumbering:  ModuleNumbering:prod 30-Sep-2020 11:34:40 CEST Run: 1 Event: 1
---
> %MSG-i ModuleNumbering:  ModuleNumbering:prod 30-Sep-2020 11:49:20 CEST Run: 1 Event: 1
55621c55621
< %MSG-i ModuleNumbering:  ModuleNumbering:prod 30-Sep-2020 11:34:40 CEST Run: 1 Event: 1
---
> %MSG-i ModuleNumbering:  ModuleNumbering:prod 30-Sep-2020 11:49:20 CEST Run: 1 Event: 1
55624,55627c55624,55627
< TimeModule> 1 1 prod ModuleNumbering 711.719
< TimeModule> 1 1 p1 PathStatusInserter 0.000180006
< TimeModule> 1 1 TriggerResults TriggerResultInserter 5.6982e-05
< ----- Begin Fatal Exception 30-Sep-2020 11:34:40 CEST-----------------------
---
> TimeModule> 1 1 prod ModuleNumbering 710.345
> TimeModule> 1 1 p1 PathStatusInserter 0.000760078
> TimeModule> 1 1 TriggerResults TriggerResultInserter 0.000238895
> ----- Begin Fatal Exception 30-Sep-2020 11:49:20 CEST-----------------------
55636c55636
< TimeEvent> 1 1 711.722
---
> TimeEvent> 1 1 710.352
55641c55641
< [1] run: 1 lumi: 1 event: 1  vsize = 811.008 deltaVsize = 0 rss = 251.285 delta = 0
---
> [1] run: 1 lumi: 1 event: 1  vsize = 811.008 deltaVsize = 0 rss = 156.164 delta = 0
55646,55647c55646,55647
< [1] run: 1 lumi: 1 event: 1  vsize = 811.008 deltaVsize = 0 rss = 251.285 delta = 0
< TimeReport> Time report complete in 716.342 seconds
---
> [1] run: 1 lumi: 1 event: 1  vsize = 811.008 deltaVsize = 0 rss = 156.164 delta = 0
> TimeReport> Time report complete in 715.492 seconds
55649,55654c55649,55654
<  - Min event:   711.722
<  - Max event:   711.722
<  - Avg event:   711.722
<  - Total loop:  711.723
<  - Total init:  4.61527
<  - Total job:   716.342
---
>  - Min event:   710.352
>  - Max event:   710.352
>  - Avg event:   710.352
>  - Total loop:  710.354
>  - Total init:  5.13328
>  - Total job:   715.492
55657c55657
<  Event Throughput: 0.00140504 ev/s
---
>  Event Throughput: 0.00140775 ev/s
55659,55660c55659,55660
<  - Total loop:  711.334
<  - Total init:  2.43249
---
>  - Total loop:  709.745
>  - Total init:  2.65475
55662c55662
<  - Total job:   713.769
---
>  - Total job:   712.403

ghugo83 · 2020-09-30T12:34:54Z

@ghugo83 - this PR #31614 should address some performance issues. Thought it may still be a premature optimisation ( "the root of all evil (or at least most of it)" ) :-)

Hmm yes DDFilteredView::get<type> still seems to take too much time time (much more than old DD).

With help of valgrind I got sth much faster by removing the call of dd4hep::dd::split in DFilteredView::get<type>
https://github.com/ianna/cmssw/blob/ac3b9b82d7854cf7da54fbde1b1f35aac9fcd36d/DetectorDescription/DDCMS/src/DDFilteredView.cc#L481

Implementation of split:
https://dd4hep.web.cern.ch/dd4hep/reference/Filter_8cpp_source.html#l00035

There is no need to have this vector to compare strings, one can just 'move along' the path we already have, until a difference is spotted.

There is also an over-use of regex: in DDFilteredView::get<type> we should only use regex when the name actually contains a regex, not for any name. But I see you also addressed this point already.

I have sth on my local which makes the run really fast, will push a PR when ready.
It only affects DDFilteredView::get<type> though, which as reported was taking most of the time.

I think in general there is an overall use of regex in DDFilteredView: probably should not, by default, convert each word of each path to regex, when not a regex anyway (keep that for the small fraction which are actually regex).

ghugo83 · 2020-09-30T12:40:03Z

@ghugo83 - FYI testing your PR #31573 + PR #31621 with the following change shows no differences:

$ git diff
diff --git a/Geometry/CMSCommonData/data/dd4hep/cmsExtendedGeometry2021.xml b/Geometry/CMSCommonData/data/dd4hep/cmsExtendedGeometry2021.xml
index 92ca1793acf..6ce3f568fec 100644
--- a/Geometry/CMSCommonData/data/dd4hep/cmsExtendedGeometry2021.xml
+++ b/Geometry/CMSCommonData/data/dd4hep/cmsExtendedGeometry2021.xml
@@ -206,7 +206,7 @@
     <Include ref='Geometry/TrackerCommonData/data/trackertec.xml'/>
     <Include ref='Geometry/TrackerCommonData/data/v2/trackerbulkhead.xml'/>
     <Include ref='Geometry/TrackerCommonData/data/trackerother.xml'/>
-    <Include ref='Geometry/TrackerCommonData/data/PhaseI/dd4hep/trackerStructureTopology.xml'/>
+    <Include ref='Geometry/TrackerCommonData/data/PhaseI/trackerStructureTopology.xml'/>
     <Include ref='Geometry/TrackerSimData/data/PhaseI/trackersens.xml'/>
     <Include ref='Geometry/TrackerRecoData/data/PhaseI/trackerRecoMaterial.xml'/>
     <Include ref='SimTracker/TrackerMaterialAnalysis/data/trackingMaterialGroups_ForPhaseI.xml'/>

11183c11183
< %MSG-i Root_Information:  ModuleNumbering:prod TGeoManager::CheckGeometry() 30-Sep-2020 11:22:49 CEST Run: 1 Event: 1
---
> %MSG-i Root_Information:  ModuleNumbering:prod TGeoManager::CheckGeometry() 30-Sep-2020 11:37:31 CEST Run: 1 Event: 1
11186c11186
< %MSG-i Root_Information:  ModuleNumbering:prod TGeoManager::CloseGeometry() 30-Sep-2020 11:22:49 CEST Run: 1 Event: 1
---
> %MSG-i Root_Information:  ModuleNumbering:prod TGeoManager::CloseGeometry() 30-Sep-2020 11:37:31 CEST Run: 1 Event: 1
11189c11189
< %MSG-i Root_Information:  ModuleNumbering:prod TGeoManager::Voxelize() 30-Sep-2020 11:22:49 CEST Run: 1 Event: 1
---
> %MSG-i Root_Information:  ModuleNumbering:prod TGeoManager::Voxelize() 30-Sep-2020 11:37:31 CEST Run: 1 Event: 1
11192c11192
< %MSG-i Root_Information:  ModuleNumbering:prod TGeoManager::CloseGeometry() 30-Sep-2020 11:22:49 CEST Run: 1 Event: 1
---
> %MSG-i Root_Information:  ModuleNumbering:prod TGeoManager::CloseGeometry() 30-Sep-2020 11:37:31 CEST Run: 1 Event: 1
11195c11195
< %MSG-i Root_Information:  ModuleNumbering:prod TGeoManager::CountLevels() 30-Sep-2020 11:22:50 CEST Run: 1 Event: 1
---
> %MSG-i Root_Information:  ModuleNumbering:prod TGeoManager::CountLevels() 30-Sep-2020 11:37:31 CEST Run: 1 Event: 1
11198c11198
< %MSG-i Root_Information:  ModuleNumbering:prod TGeoManager::CloseGeometry() 30-Sep-2020 11:22:50 CEST Run: 1 Event: 1
---
> %MSG-i Root_Information:  ModuleNumbering:prod TGeoManager::CloseGeometry() 30-Sep-2020 11:37:31 CEST Run: 1 Event: 1
11201c11201
< %MSG-i Root_Information:  ModuleNumbering:prod TGeoManager::CloseGeometry() 30-Sep-2020 11:22:50 CEST Run: 1 Event: 1
---
> %MSG-i Root_Information:  ModuleNumbering:prod TGeoManager::CloseGeometry() 30-Sep-2020 11:37:31 CEST Run: 1 Event: 1
11206c11206
< %MSG-i TrackerGeom:  ModuleNumbering:prod 30-Sep-2020 11:22:50 CEST Run: 1 Event: 1
---
> %MSG-i TrackerGeom:  ModuleNumbering:prod 30-Sep-2020 11:37:31 CEST Run: 1 Event: 1
11209c11209
< %MSG-i TrackerTopologyEP:  ModuleNumbering:prod 30-Sep-2020 11:22:50 CEST Run: 1 Event: 1
---
> %MSG-i TrackerTopologyEP:  ModuleNumbering:prod 30-Sep-2020 11:37:31 CEST Run: 1 Event: 1
11212c11212
< %MSG-i ModuleNumbering:  ModuleNumbering:prod 30-Sep-2020 11:22:50 CEST Run: 1 Event: 1
---
> %MSG-i ModuleNumbering:  ModuleNumbering:prod 30-Sep-2020 11:37:31 CEST Run: 1 Event: 1
55604,55605c55604,55605
< %MSG-i ModuleNumbering:  ModuleNumbering:prod 30-Sep-2020 11:34:40 CEST Run: 1 Event: 1
<  Top node is  0x7ff3a3fc34c0 Tracker
---
> %MSG-i ModuleNumbering:  ModuleNumbering:prod 30-Sep-2020 11:49:20 CEST Run: 1 Event: 1
>  Top node is  0x7fcb6ed824c0 Tracker
55607c55607
< %MSG-i ModuleNumbering:  ModuleNumbering:prod 30-Sep-2020 11:34:40 CEST Run: 1 Event: 1
---
> %MSG-i ModuleNumbering:  ModuleNumbering:prod 30-Sep-2020 11:49:20 CEST Run: 1 Event: 1
55618c55618
< %MSG-i ModuleNumbering:  ModuleNumbering:prod 30-Sep-2020 11:34:40 CEST Run: 1 Event: 1
---
> %MSG-i ModuleNumbering:  ModuleNumbering:prod 30-Sep-2020 11:49:20 CEST Run: 1 Event: 1
55621c55621
< %MSG-i ModuleNumbering:  ModuleNumbering:prod 30-Sep-2020 11:34:40 CEST Run: 1 Event: 1
---
> %MSG-i ModuleNumbering:  ModuleNumbering:prod 30-Sep-2020 11:49:20 CEST Run: 1 Event: 1
55624,55627c55624,55627
< TimeModule> 1 1 prod ModuleNumbering 711.719
< TimeModule> 1 1 p1 PathStatusInserter 0.000180006
< TimeModule> 1 1 TriggerResults TriggerResultInserter 5.6982e-05
< ----- Begin Fatal Exception 30-Sep-2020 11:34:40 CEST-----------------------
---
> TimeModule> 1 1 prod ModuleNumbering 710.345
> TimeModule> 1 1 p1 PathStatusInserter 0.000760078
> TimeModule> 1 1 TriggerResults TriggerResultInserter 0.000238895
> ----- Begin Fatal Exception 30-Sep-2020 11:49:20 CEST-----------------------
55636c55636
< TimeEvent> 1 1 711.722
---
> TimeEvent> 1 1 710.352
55641c55641
< [1] run: 1 lumi: 1 event: 1  vsize = 811.008 deltaVsize = 0 rss = 251.285 delta = 0
---
> [1] run: 1 lumi: 1 event: 1  vsize = 811.008 deltaVsize = 0 rss = 156.164 delta = 0
55646,55647c55646,55647
< [1] run: 1 lumi: 1 event: 1  vsize = 811.008 deltaVsize = 0 rss = 251.285 delta = 0
< TimeReport> Time report complete in 716.342 seconds
---
> [1] run: 1 lumi: 1 event: 1  vsize = 811.008 deltaVsize = 0 rss = 156.164 delta = 0
> TimeReport> Time report complete in 715.492 seconds
55649,55654c55649,55654
<  - Min event:   711.722
<  - Max event:   711.722
<  - Avg event:   711.722
<  - Total loop:  711.723
<  - Total init:  4.61527
<  - Total job:   716.342
---
>  - Min event:   710.352
>  - Max event:   710.352
>  - Avg event:   710.352
>  - Total loop:  710.354
>  - Total init:  5.13328
>  - Total job:   715.492
55657c55657
<  Event Throughput: 0.00140504 ev/s
---
>  Event Throughput: 0.00140775 ev/s
55659,55660c55659,55660
<  - Total loop:  711.334
<  - Total init:  2.43249
---
>  - Total loop:  709.745
>  - Total init:  2.65475
55662c55662
<  - Total job:   713.769
---
>  - Total job:   712.403

Ok cool. Was in FilteredView speed, will switch back to that branch. Need to compare what is stored in GeometricDet etc, but sounds good :)

ianna · 2020-09-30T12:47:14Z

@ghugo83 - also the last merged PR #31614 gives a 3x speedup:

<  - Min event:   220.235
<  - Max event:   220.235
<  - Avg event:   220.235
<  - Total loop:  220.237
<  - Total init:  4.07409
<  - Total job:   224.312
---
>  - Min event:   711.722
>  - Max event:   711.722
>  - Avg event:   711.722
>  - Total loop:  711.723
>  - Total init:  4.61527
>  - Total job:   716.342

ianna · 2020-09-30T13:23:12Z

@ghugo83 - using string iteration instead of dd4hep::split is slightly better :-)

<  - Min event:   199.23
<  - Max event:   199.23
<  - Avg event:   199.23
<  - Total loop:  199.231
<  - Total init:  4.13025
<  - Total job:   203.363
---
>  - Min event:   711.722
>  - Max event:   711.722
>  - Avg event:   711.722
>  - Total loop:  711.723
>  - Total init:  4.61527
>  - Total job:   716.342

ghugo83 · 2020-09-30T14:44:05Z

@ianna yes with x3 and x3 we might hopepfully eventually join sth similar as old DD FilteredView (there was a x10 discrepancy if I recall properly, really painful for tests cycles and so on) :)
Sth interesting is that there is still a point in DDFilteredView::firstChild(), once the full tree has been processed, where there is a very long stop. It might be interesting to look at that as well.

ghugo83 · 2020-09-30T14:44:42Z

Also this is useful cause will be needed for the other subdetectors as well anyway.

ianna · 2020-09-30T14:53:10Z

@ianna yes with x3 and x3 we might hopepfully eventually join sth similar as old DD FilteredView (there was a x10 discrepancy if I recall properly, really painful for tests cycles and so on) :)
Sth interesting is that there is still a point in DDFilteredView::firstChild(), once the full tree has been processed, where there is a very long stop. It might be interesting to look at that as well.

Indeed, I've got 10x improvement on your PR test #31630
I'm checking with other sub detectors if it made as a dramatic effect as on Tracker numbering. User stories are different...

A break for the firstChild is in the works, though most of the sub detectors use level to break out of it already :-)

ghugo83 · 2020-09-30T15:00:21Z

Haaa I see at #31630 you pushed it already haha
Ok cool

Maybe then also worth it to minimize regex usage + avoid dd4hep::dd::split whenever possible in the rest of the FilteredView

cmsbuild added the pending-assignment label Sep 25, 2020

ianna mentioned this issue Sep 25, 2020

DD4hep: Introduces functional Tracker navigation + DetIds building #31573

Merged

cmsbuild added geometry-pending pending-signatures and removed pending-assignment labels Sep 25, 2020

cmsbuild added the urgent label Sep 25, 2020

ianna mentioned this issue Sep 29, 2020

[DD4hep] Remove ".*:" aka "any" Namespace Definition from a Part Selector #31610

Merged

ianna mentioned this issue Sep 29, 2020

[DD4hep] Allow Namespace Comparison in a Part Selector #31614

Merged

ianna closed this as completed Sep 29, 2020

cmsbuild added fully-signed and removed geometry-pending pending-signatures labels Sep 29, 2020

cmsbuild added the geometry-approved label Sep 29, 2020

[DD4hep] Allow Use of Namespace in SpecPar Comparison #31579

[DD4hep] Allow Use of Namespace in SpecPar Comparison #31579

Comments

ianna commented Sep 25, 2020

cmsbuild commented Sep 25, 2020

ianna commented Sep 25, 2020

cmsbuild commented Sep 25, 2020

ianna commented Sep 25, 2020

cvuosalo commented Sep 25, 2020

cvuosalo commented Sep 28, 2020

ianna commented Sep 28, 2020

ghugo83 commented Sep 28, 2020

ghugo83 commented Sep 28, 2020 • edited

ianna commented Sep 28, 2020

ghugo83 commented Sep 28, 2020 • edited

ianna commented Sep 29, 2020

ghugo83 commented Sep 29, 2020

ghugo83 commented Sep 29, 2020

ianna commented Sep 29, 2020

ianna commented Sep 29, 2020

ianna commented Sep 29, 2020

ianna commented Sep 29, 2020

cmsbuild commented Sep 29, 2020

ghugo83 commented Sep 29, 2020 • edited

ghugo83 commented Sep 29, 2020 • edited

ghugo83 commented Sep 29, 2020 • edited

ianna commented Sep 30, 2020

ianna commented Sep 30, 2020

ianna commented Sep 30, 2020

ghugo83 commented Sep 30, 2020 • edited

ianna commented Sep 30, 2020

ghugo83 commented Sep 30, 2020 • edited

ghugo83 commented Sep 30, 2020 • edited

ianna commented Sep 30, 2020

ianna commented Sep 30, 2020

ghugo83 commented Sep 30, 2020

ghugo83 commented Sep 30, 2020

ianna commented Sep 30, 2020

ghugo83 commented Sep 30, 2020

ghugo83 commented Sep 28, 2020 •

edited

ghugo83 commented Sep 28, 2020 •

edited

ghugo83 commented Sep 29, 2020 •

edited

ghugo83 commented Sep 29, 2020 •

edited

ghugo83 commented Sep 29, 2020 •

edited

ghugo83 commented Sep 30, 2020 •

edited

ghugo83 commented Sep 30, 2020 •

edited

ghugo83 commented Sep 30, 2020 •

edited