[DD4hep] Navigation Performance

[ianna]

Study by how much it can be improved before the following issue is resolved:
https://sft.its.cern.ch/jira/browse/ROOT-9742

[ianna]

assign geometry

[cmsbuild]

New categories assigned: geometry

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

[cmsbuild]

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]

@dpiparo - perhaps, CMS could ping ROOT team and discuss their priorities at an appropriate forum? Thanks.

[davidlange6]

What is the outstanding issue? It’s not clear from the Jira David On 27 Sep 2020, at 10:16, Ianna Osborne <notifications@github.com> wrote:  @dpiparo<https://github.com/dpiparo> - perhaps, CMS could ping ROOT team and discuss their priorities at an appropriate forum? Thanks. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub<#31594 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABGPFQ23QPAD756EPM7O2ZTSH3YEHANCNFSM4R3NFCZA>.

[ianna]

@davidlange6 - it looks like only no 1 was addressed.

Quote:

Some geometry tasks were identified during initial discussions with LHCb and CMS, related to integration of DD4HEP in their DD software.

1. Support for reading/writing optical surface properties from/to GDML in a similar way as in Geant4. This will complete the conversion circle TGeo <->Geant4 via GDML
2. Support for building geometry in parallel. This requires introduction in a first phase of thread-local geometry managers and accessors to those used internally by TGeo instead of the global gGeoManager pointer
3. Support for wildcard queries on top of the geometry tree, providing callbacks to user code for filtering geometry information
4. Long term evolution to having more stateless geometry information, allowing to decouple creation of objects with their central bookkeeping (no need for geometry singleton)

[makortel]

@ianna For my education, about point 2, would that be about running internals of a single geometry ESProducer concurrently, or running multiple (different) geometry ESProducers concurrently?

[ianna]

@ianna For my education, about point 2, would that be about running internals of a single geometry ESProducer concurrently, or running multiple (different) geometry ESProducers concurrently?

@makortel - We would use the former to processes multiple XML files or fragments with a tbb task per file, for example. We would use the latter to run the Reco geometry builders concurrently.

[makortel]

For my education, about point 2, would that be about running internals of a single geometry ESProducer concurrently, or running multiple (different) geometry ESProducers concurrently?

We would use the former to processes multiple XML files or fragments with a tbb task per file, for example. We would use the latter to run the Reco geometry builders concurrently.

Right. I can imagine the point 2 to be required for the former, but is it needed for the latter as well?

[ianna]

For my education, about point 2, would that be about running internals of a single geometry ESProducer concurrently, or running multiple (different) geometry ESProducers concurrently?

We would use the former to processes multiple XML files or fragments with a tbb task per file, for example. We would use the latter to run the Reco geometry builders concurrently.

Right. I can imagine the point 2 to be required for the former, but is it needed for the latter as well?

Not yet, so fare we use the TGeo as read-only.

[makortel]

For my education, about point 2, would that be about running internals of a single geometry ESProducer concurrently, or running multiple (different) geometry ESProducers concurrently?

We would use the former to processes multiple XML files or fragments with a tbb task per file, for example. We would use the latter to run the Reco geometry builders concurrently.

Right. I can imagine the point 2 to be required for the former, but is it needed for the latter as well?

Not yet, so fare we use the TGeo as read-only.

Thanks. I was asking because "soon" we will be doing the latter (#31061). There is a way to declare that an ESProducer uses a "shared, non-thread-safe resource" in which case the framework will run only one ESProducer using such a resource at a time. If it happens in the future that some ESProducer would need a write access to the TGeo (or whatever else non-thread-safe in DD4hep), we need to look into that.

[civanch]

@ianna , the requirement to read xml files in parallel should not be an urgent requirement, because old DDD is not parallel as well. In a long term, it may be a valid requirement but for today we need the minimum: DD4Hep should provide the same results as old DDD. If we will face the situation, that DD4Hep is several times slower than DDD we may accept this if geometry is built correctly for Run-3. There is enough time after 11_2 to improve performance.

[davidlange6]

I added a generic comment in the AF meeting today.

[ianna]

I added a generic comment in the AF meeting today.

@davidlange6 - Thanks! It would be good if they replied to the Jira issue and commented on what is a timescale for implementing thread-local geometry managers? If they are planning to provide a wildcard query and if singleton is going away any time soon? Thanks again!

[ianna]

TrackerModuleNumbering test from DD reference number is 5 sec in total

DD4hep XML parsing and a DDCompactView producer is around 10 sec
This is included in the TrackerModuleNumbering test numbers that show improvement from:

• Current CMSSW_11_2_X_2020-10-06-2300 IB:

%MSG-i ModuleNumbering:  ModuleNumbering:prod 07-Oct-2020 10:27:30 CEST Run: 1 Event: 1
 And Contains  Daughters: 17004
%MSG
TimeModule> 1 1 prod ModuleNumbering 128.871

• avoiding double search for the attributes that are defined in the same specpar by prefetching it:

radLength_ = fv->get<double>("TrackerRadLength");
xi_ = fv->getNextValue("TrackerXi");

%MSG-i ModuleNumbering:  ModuleNumbering:prod 07-Oct-2020 10:47:03 CEST Run: 1 Event: 1
 And Contains  Daughters: 17004
%MSG
TimeModule> 1 1 prod ModuleNumbering 94.9412

• avoiding regexp comparison where possible:

%MSG-i ModuleNumbering:  ModuleNumbering:prod 07-Oct-2020 16:11:18 CEST Run: 1 Event: 1
 And Contains  Daughters: 17004
%MSG
TimeModule> 1 1 prod ModuleNumbering 43.474

• replacing the rest of tbb containers with regular std containers

[ianna]

+1

bottlenecks are understood, the units migration should happen first

[cmsbuild]

This issue is fully signed and ready to be closed.