MTD geometry: speed up MTD reco geometry construction #43124

fabiocos · 2023-10-26T15:49:36Z

PR description:

The DDD implementation of the MTD reconstruction geometry creation, in the DDCmsMTDConstruction class of Geometry/MTDNumberingBuilder is a killer for the startup CMSSW performances. It turns out that the veto filtering on non sensitive volumes is the key responsible for this, likely due to an excessive number of find of strings in the logical name of processed volumes. Just removing it, without impact on the geometry reconstructed, effectively solves the problem.

As a side note, the DD4hep version, implemented in parallel since 2020, does not suffer of this issue, using a different filtering approach. Unfortunately this is of little help, since we are still in the middle of the transition between different geometry back-ends.

Addressing #43062 .

PR validation:

The MTD reconstruction geometry unit tests pass, showing that the same geometry as before is reconstructed. Both Tracer timing and igprof profiling show a sizeable improvement, from almost 50 down to 3 seconds on the machine used for tests.

Before:

Begin processing the 1st record. Run 1, Event 1, LumiSection 1 on stream 0 at 26-Oct-2023 17:03:28.265 CEST
26-Oct-2023 17:03:28.26 CEST  ++++ starting: processing event : stream = 0 run = 1 lumi = 1 event = 1 time = 1
26-Oct-2023 17:03:28.26 CEST  ++++++ starting: processing path 'p1' : stream = 0
26-Oct-2023 17:03:28.26 CEST  ++++++++ starting: prefetching before processing event for module: stream = 0 label = 'prod' id = 3
26-Oct-2023 17:03:28.26 CEST  ++++++++ starting: prefetching for esmodule: label = 'mtdNumberingGeometry' type = MTDGeometricTimingDetESModule in record = IdealGeometryRecord
26-Oct-2023 17:03:28.26 CEST  ++++++++++ starting: prefetching for esmodule: label = '' type = XMLIdealGeometryESSource in record = IdealGeometryRecord
26-Oct-2023 17:03:28.26 CEST  ++++++++++ finished: prefetching for esmodule: label = '' type = XMLIdealGeometryESSource in record = IdealGeometryRecord
26-Oct-2023 17:03:28.26 CEST  ++++++++++ starting: processing esmodule: label = '' type = XMLIdealGeometryESSource in record = IdealGeometryRecord
26-Oct-2023 17:03:31.47 CEST  ++++++++++ finished: processing esmodule: label = '' type = XMLIdealGeometryESSource in record = IdealGeometryRecord
26-Oct-2023 17:03:31.47 CEST  ++++++++ finished: prefetching for esmodule: label = 'mtdNumberingGeometry' type = MTDGeometricTimingDetESModule in record = IdealGeometryRecord
26-Oct-2023 17:03:31.47 CEST  ++++++++ starting: processing esmodule: label = 'mtdNumberingGeometry' type = MTDGeometricTimingDetESModule in record = IdealGeometryRecord
26-Oct-2023 17:04:08.03 CEST  ++++++++ finished: processing esmodule: label = 'mtdNumberingGeometry' type = MTDGeometricTimingDetESModule in record = IdealGeometryRecord
26-Oct-2023 17:04:08.03 CEST  ++++++++ finished: prefetching before processing event for module: stream = 0 label = 'prod' id = 3
26-Oct-2023 17:04:08.03 CEST  ++++++++ starting: processing event for module: stream = 0 label = 'prod' id = 3
%MSG-i GeometricTimingDetAnalyzer:  GeometricTimingDetAnalyzer:prod  26-Oct-2023 17:04:08 CEST Run: 1 Event: 1
Beginning MTD GeometricTimingDet container dump 
%MSG

With this PR:

Begin processing the 1st record. Run 1, Event 1, LumiSection 1 on stream 0 at 26-Oct-2023 17:22:07.701 CEST
26-Oct-2023 17:22:07.70 CEST  ++++ starting: processing event : stream = 0 run = 1 lumi = 1 event = 1 time = 1
26-Oct-2023 17:22:07.70 CEST  ++++++ starting: processing path 'p1' : stream = 0
26-Oct-2023 17:22:07.70 CEST  ++++++++ starting: prefetching before processing event for module: stream = 0 label = 'prod' id = 3
26-Oct-2023 17:22:07.70 CEST  ++++++++ starting: prefetching for esmodule: label = 'mtdNumberingGeometry' type = MTDGeometricTimingDetESModule in record = IdealGeometryRecord
26-Oct-2023 17:22:07.70 CEST  ++++++++++ starting: prefetching for esmodule: label = '' type = XMLIdealGeometryESSource in record = IdealGeometryRecord
26-Oct-2023 17:22:07.70 CEST  ++++++++++ finished: prefetching for esmodule: label = '' type = XMLIdealGeometryESSource in record = IdealGeometryRecord
26-Oct-2023 17:22:07.70 CEST  ++++++++++ starting: processing esmodule: label = '' type = XMLIdealGeometryESSource in record = IdealGeometryRecord
26-Oct-2023 17:22:10.70 CEST  ++++++++++ finished: processing esmodule: label = '' type = XMLIdealGeometryESSource in record = IdealGeometryRecord
26-Oct-2023 17:22:10.70 CEST  ++++++++ finished: prefetching for esmodule: label = 'mtdNumberingGeometry' type = MTDGeometricTimingDetESModule in record = IdealGeometryRecord
26-Oct-2023 17:22:10.70 CEST  ++++++++ starting: processing esmodule: label = 'mtdNumberingGeometry' type = MTDGeometricTimingDetESModule in record = IdealGeometryRecord
%MSG-i MTDNumbering:   MTDGeometricTimingDetESModule:mtdNumberingGeometry@callESModule  26-Oct-2023 17:22:10 CEST Run: 1 Event: 1
Top level node = OCMS
%MSG

…o geometry construction

cmsbuild · 2023-10-26T15:56:11Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-43124/37386

This PR adds an extra 12KB to repository

cmsbuild · 2023-10-26T15:56:36Z

A new Pull Request was created by @fabiocos (Fabio Cossutti) for master.

It involves the following packages:

Geometry/MTDNumberingBuilder (geometry, upgrade)

@Dr15Jones, @mdhildreth, @cmsbuild, @srimanob, @makortel, @civanch, @AdrianoDee, @bsunanda can you please review it and eventually sign? Thanks.
@bsunanda this is something you requested to watch as well.
@antoniovilela, @sextonkennedy, @rappoccio you are the release manager for this.

cms-bot commands are listed here

fabiocos · 2023-10-26T15:56:47Z

please test

srimanob · 2023-10-26T16:17:56Z

Geometry/MTDNumberingBuilder/plugins/DDCmsMTDConstruction.cc

@@ -1,5 +1,3 @@
-//#define EDM_ML_DEBUG


Do you want to remove also this comment of debug? Because I still see #ifdef EDM_ML_DEBUG in the code.

I would guess there is nothing left to test, next time it may be added.

Not sure I understand. I see, i.e.

#ifdef EDM_ML_DEBUG edm::LogVerbatim("MTDNumbering") << "Module = " << fv.name() << " fullNode = " << fullNode << " thisNode = " << thisNode; #endif

in the code. It is not a big deal for me. I just want to confirm if this is by intention.

Just to note that the preferred way is to define the EDM_ML_DEBUG macro at the compilation command

USER_CXXFLAGS="-DEDM_ML_DEBUG" scram b

https://twiki.cern.ch/twiki/bin/view/CMSPublic/SWGuideMessageLogger#LogDebug

(that doesn't provide file-level control, but the message categories are the intended ones for finer-grained control)

I very often use file-level control, and in the past I was used to add commented #define EDM_ML_DEBUG statements in many places. But since the file needs anyway to be edited, adding the statement directly when needed does not really make any difference, and the code looks cleaner.

cmsbuild · 2023-10-26T18:30:07Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-cba403/35440/summary.html
COMMIT: a427727
CMSSW: CMSSW_13_3_X_2023-10-26-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/43124/35440/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

You potentially removed 2 lines from the logs
Reco comparison results: 102 differences found in the comparisons
DQMHistoTests: Total files compared: 50
DQMHistoTests: Total histograms compared: 3357400
DQMHistoTests: Total failures: 1310
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3356068
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 49 files compared)
Checked 214 log files, 167 edm output root files, 50 DQM output files
TriggerResults: no differences found

srimanob · 2023-10-26T18:58:36Z

Hi @fabiocos
Is the failure in Phase-2 workflow test of MTD expected because of this PR?
https://cmssdt.cern.ch/SDT/jenkins-artifacts/baseLineComparisons/CMSSW_13_3_X_2023-10-26-1100+cba403/59434/23234.0_TTbar_14TeV+2026D94/MTD_Vertices.html

bsunanda · 2023-10-26T21:36:15Z

+geometry

fabiocos · 2023-10-27T07:39:56Z

@srimanob no, I would not expect a change while processing identical input samples, especially since the unit tests are ok (comparing the dump of DetIds and corresponding reference positions). I need to have a closer look to understand

fabiocos · 2023-10-27T07:40:03Z

hold

cmsbuild · 2023-10-27T07:40:16Z

Pull request has been put on hold by @fabiocos
They need to issue an unhold command to remove the hold state or L1 can unhold it for all

srimanob · 2023-10-27T08:15:05Z

assign mtd

felicepantaleo · 2023-10-27T09:15:28Z

thanks @fabiocos for addressing this problem !

fabiocos · 2023-10-27T16:11:56Z

unhold

cmsbuild · 2023-10-27T19:30:50Z

-1

Failed Tests: RelVals-INPUT
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-cba403/35472/summary.html
COMMIT: 3a7667e
CMSSW: CMSSW_13_3_X_2023-10-27-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/43124/35472/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals-INPUT

4.764.76_ZMuSkim2012D/step2_ZMuSkim2012D.log

Comparison Summary

Summary:

You potentially removed 1 lines from the logs
Reco comparison results: 4 differences found in the comparisons
DQMHistoTests: Total files compared: 50
DQMHistoTests: Total histograms compared: 3357400
DQMHistoTests: Total failures: 3
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3357375
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 49 files compared)
Checked 214 log files, 167 edm output root files, 50 DQM output files
TriggerResults: no differences found

civanch · 2023-10-28T10:13:06Z

+1

all PRs fail with 4.76 WF since yesterday. I would merge this one, because it is absolutely unrelated and there is no sense to re-request tests.

civanch · 2023-10-29T07:47:28Z

please test

cmsbuild · 2023-10-29T10:15:38Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-cba403/35485/summary.html
COMMIT: 3a7667e
CMSSW: CMSSW_13_3_X_2023-10-29-0000/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/43124/35485/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

You potentially removed 9 lines from the logs
Reco comparison results: 14 differences found in the comparisons
DQMHistoTests: Total files compared: 50
DQMHistoTests: Total histograms compared: 3362691
DQMHistoTests: Total failures: 12
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3362657
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 49 files compared)
Checked 214 log files, 167 edm output root files, 50 DQM output files
TriggerResults: no differences found

fabiocos · 2023-11-06T08:43:45Z

@srimanob is this PR ok for you, or do you see residual issues?

srimanob · 2023-11-06T08:44:47Z

+Upgrade

cmsbuild · 2023-11-06T08:45:11Z

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

antoniovilela · 2023-11-06T10:29:55Z

+1

Remove veto filtering in DDCmsMTDConstruction for DDD to speed up rec…

a427727

…o geometry construction

cmsbuild added this to the CMSSW_13_3_X milestone Oct 26, 2023

cmsbuild added geometry-pending pending-signatures tests-pending orp-pending upgrade-pending code-checks-pending labels Oct 26, 2023

fabiocos mentioned this pull request Oct 26, 2023

Long startup time for simplest workflows #43062

Open

cmsbuild added code-checks-approved and removed code-checks-pending labels Oct 26, 2023

cmsbuild added tests-started and removed tests-pending labels Oct 26, 2023

srimanob reviewed Oct 26, 2023

View reviewed changes

cmsbuild added tests-approved and removed tests-started labels Oct 26, 2023

cmsbuild added geometry-approved and removed geometry-pending labels Oct 26, 2023

cmsbuild added the hold label Oct 27, 2023

cmsbuild removed the hold label Oct 27, 2023

cmsbuild added tests-rejected and removed tests-started labels Oct 27, 2023

cmsbuild added geometry-approved and removed geometry-pending labels Oct 28, 2023

cmsbuild added tests-started and removed tests-rejected labels Oct 29, 2023

cmsbuild added tests-approved and removed tests-started labels Oct 29, 2023

cmsbuild added fully-signed upgrade-approved and removed pending-signatures upgrade-pending labels Nov 6, 2023

cmsbuild added orp-approved and removed orp-pending labels Nov 6, 2023

cmsbuild merged commit 5915557 into cms-sw:master Nov 6, 2023

fabiocos deleted the fc-speedupMtdNumbering branch November 7, 2023 10:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MTD geometry: speed up MTD reco geometry construction #43124

MTD geometry: speed up MTD reco geometry construction #43124

fabiocos commented Oct 26, 2023 •

edited

Loading

cmsbuild commented Oct 26, 2023

cmsbuild commented Oct 26, 2023

fabiocos commented Oct 26, 2023

srimanob Oct 26, 2023

civanch Oct 26, 2023

srimanob Oct 26, 2023

makortel Oct 26, 2023

fabiocos Oct 26, 2023

cmsbuild commented Oct 26, 2023

srimanob commented Oct 26, 2023

bsunanda commented Oct 26, 2023

fabiocos commented Oct 27, 2023

fabiocos commented Oct 27, 2023

cmsbuild commented Oct 27, 2023

srimanob commented Oct 27, 2023

felicepantaleo commented Oct 27, 2023

fabiocos commented Oct 27, 2023

cmsbuild commented Oct 27, 2023

civanch commented Oct 28, 2023

civanch commented Oct 29, 2023

cmsbuild commented Oct 29, 2023

fabiocos commented Nov 6, 2023

srimanob commented Nov 6, 2023

cmsbuild commented Nov 6, 2023

antoniovilela commented Nov 6, 2023

MTD geometry: speed up MTD reco geometry construction #43124

MTD geometry: speed up MTD reco geometry construction #43124

Conversation

fabiocos commented Oct 26, 2023 • edited Loading

PR description:

PR validation:

cmsbuild commented Oct 26, 2023

cmsbuild commented Oct 26, 2023

fabiocos commented Oct 26, 2023

srimanob Oct 26, 2023

Choose a reason for hiding this comment

civanch Oct 26, 2023

Choose a reason for hiding this comment

srimanob Oct 26, 2023

Choose a reason for hiding this comment

makortel Oct 26, 2023

Choose a reason for hiding this comment

fabiocos Oct 26, 2023

Choose a reason for hiding this comment

cmsbuild commented Oct 26, 2023

Comparison Summary

srimanob commented Oct 26, 2023

bsunanda commented Oct 26, 2023

fabiocos commented Oct 27, 2023

fabiocos commented Oct 27, 2023

cmsbuild commented Oct 27, 2023

srimanob commented Oct 27, 2023

felicepantaleo commented Oct 27, 2023

fabiocos commented Oct 27, 2023

cmsbuild commented Oct 27, 2023

RelVals-INPUT

Comparison Summary

civanch commented Oct 28, 2023

civanch commented Oct 29, 2023

cmsbuild commented Oct 29, 2023

Comparison Summary

fabiocos commented Nov 6, 2023

srimanob commented Nov 6, 2023

cmsbuild commented Nov 6, 2023

antoniovilela commented Nov 6, 2023

fabiocos commented Oct 26, 2023 •

edited

Loading