Optimize MagGeoBuilder

[Dr15Jones]

PR description:

• Avoid extra work done for provenance tracking when looking up magnetic field related files.

The IB performance measurements for the NANO step shows that initializing the MagField object takes the majority of the startup time for the job.

PR validation:

Code compiles and performance measurements show an improvement.

[Dr15Jones]

Before the change we see

            3.6  .........       1.45 / 1.62         magneticfield::MagGeoBuilder::build(cms::DDDetector const*) [261]
[296]       3.6       1.45       0.00 / 1.45       magneticfield::MagGeoBuilder::buildInterpolator(magneticfield::volumeHandle const*, std::map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, MagneticFieldProvider<float>*, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, MagneticFieldProvider<float>*> > >&)
            2.2  .........       0.88 / 0.91         edm::FileInPath::FileInPath(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) [429]
            1.4  .........       0.55 / 0.55         MFGridFactory::build(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, GloballyPositioned<float> const&) [646]

after

            3.3  .........       1.30 / 1.46         magneticfield::MagGeoBuilder::build(cms::DDDetector const*) [273]
[324]       3.3       1.30       0.02 / 1.28       magneticfield::MagGeoBuilder::buildInterpolator(magneticfield::volumeHandle const*, std::map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, MagneticFieldProvider<float>*, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, MagneticFieldProvider<float>*> > >&)
            1.9  .........       0.76 / 0.76         MFGridFactory::build(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, GloballyPositioned<float> const&) [478]
            1.3  .........       0.49 / 0.49         edm::FileInPath::findFile(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) [680]

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37934/29982

• This PR adds an extra 28KB to repository

[cmsbuild]

A new Pull Request was created by @Dr15Jones (Chris Jones) for master.

It involves the following packages:

• FWCore/Utilities (core)
• MagneticField/GeomBuilder (reconstruction)

@smuzaffar, @Dr15Jones, @makortel, @clacaputo, @cmsbuild, @slava77, @jpata can you please review it and eventually sign? Thanks.
@makortel, @felicepantaleo, @namapane, @wddgit this is something you requested to watch as well.
@perrotta, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

[Dr15Jones]

please test

[Dr15Jones]

The underlying performance problem is MagGeoBuilder reads 9178 small files. E.g.

/cvmfs/cms-ib.cern.ch/nweek-02732/slc7_amd64_gcc10/cms/cmssw-patch/CMSSW_12_4_X_2022-05-10-1100/external/slc7_amd64_gcc10/data/MagneticField/Interpolation/data/grid_160812_3_8t/s10/grid.1049.bin

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37934/29984

• This PR adds an extra 32KB to repository

[cmsbuild]

Pull request #37934 was updated. @smuzaffar, @Dr15Jones, @makortel, @clacaputo, @cmsbuild, @slava77, @jpata can you please check and sign again.

[Dr15Jones]

please test

[cmsbuild]

Pull request #37934 was updated. @smuzaffar, @Dr15Jones, @makortel, @clacaputo, @cmsbuild, @slava77, @jpata can you please check and sign again.

[Dr15Jones]

Please test

[cmsbuild]

-1

Failed Tests: UnitTests
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3180dd/24712/summary.html
COMMIT: e861c6b
CMSSW: CMSSW_12_4_X_2022-05-13-1100/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/37934/24712/install.sh to create a dev area with all the needed externals and cmssw changes.

Unit Tests

I found errors in the following unit tests:

---> test SiStripDAQ_O2O_test had ERRORS

Comparison Summary

Summary:

• No significant changes to the logs found
• Reco comparison results: 2 differences found in the comparisons
• DQMHistoTests: Total files compared: 49
• DQMHistoTests: Total histograms compared: 3697377
• DQMHistoTests: Total failures: 92
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 3697263
• DQMHistoTests: Total skipped: 22
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
• Checked 206 log files, 45 edm output root files, 49 DQM output files
• TriggerResults: no differences found

[Dr15Jones]

The unit test failure is also happening in the IBs

[Dr15Jones]

ping

the unit test failure is from the IB.

[makortel]

+core

[jpata]

Was there any impact on memory usage?

[Dr15Jones]

Was there any impact on memory usage?

This is essentially just a refactoring of the code without any changes to any memory structures. As such there is no reason for the memory footprint to have changed.

[jpata]

+reconstruction

• technical, speeds up loading the magnetic fields

[cmsbuild]

This pull request is fully signed and it will be integrated in one of the next master IBs (but tests are reportedly failing). This pull request will now be reviewed by the release team before it's merged. @perrotta, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

[jpata]

This is essentially just a refactoring of the code without any changes to any memory structures. As such there is no reason for the memory footprint to have changed.

Got it. On a quick read, I had the impression you are caching the magnetic fields, but I missed the point.

[qliphy]

+1

[qliphy]

merge