Magnetic Field optimization #29556

namapane · 2020-04-24T11:36:15Z

PR description:

Minor optimizations discussed in Fix a scaling issue in MagneticField/VolumeBasedEngine #28180 (some computations moved from MF query calls to constructor)
move if(debug) couts to MessageLogger
some minor cleanup of deprecated, obsoleted or unused stuff.

PR validation:

Regression-tested for identical values against reference files for all maps/currents/eras.

Optimizations found to sum up to a 0.5% speedup for non-cached MF calls, according to callgrind.

…onstruction time. Improves 0.4% CPU for non-cached calls. Also migrate debug couts to MessageLogger

cmsbuild · 2020-04-24T11:36:43Z

The code-checks are being triggered in jenkins.

cmsbuild · 2020-04-24T12:01:21Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-29556/14848

This PR adds an extra 56KB to repository

Code check has found code style and quality issues which could be resolved by applying following patch(s)

code-checks:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-29556/14848/code-checks.patch
e.g. curl https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-29556/14848/code-checks.patch | patch -p1
You can also run scram build code-checks to apply code checks directly

slava77 · 2020-04-24T12:10:27Z

MagneticField/VolumeBasedEngine/src/MagGeometry.cc

+  switch (geomVersion >= 120812 ? 0 : (geomVersion >= 90812 ? 1 : 2)) {
+    case 0:  // since 120812
+      barrelRsq1 = 172.400f * 172.400f;
+      barrelRsq2 = 308.735f * 308.735f;
+      barrelZ0 = 350.000f;
+      barrelZ1 = 633.290f;
+      barrelZ2 = 662.010f;
+      break;
+    case 1:  // version 90812 (no longer in use)
+      barrelRsq1 = 172.400f * 172.400f;
+      barrelRsq2 = 308.755f * 308.755f;
+      barrelZ0 = 350.000f;
+      barrelZ1 = 633.890f;
+      barrelZ2 = 662.010f;
+      break;
+    case 2:  // versions 71212, 90322
+      barrelRsq1 = 172.400f * 172.400f;
+      barrelRsq2 = 308.755f * 308.755f;
+      barrelZ0 = 350.000f;
+      barrelZ1 = 633.290f;
+      barrelZ2 = 661.010f;
+      break;


this is apparently relocating some magic numbers from what was earlier in MagGeometry::inBarrel.
There we had // FIXME: Get these dimensions from the builder.
Is this available from the builder?
Minimally, it's still worth to keep the FIXME comment.

I put the comment back,
I wrote the FIXME a dozen years ago in the hope I would have figured out a reasonable way to detect the dimensions automatically or to label volumes appropriately in the geometry. I haven't since. A sufficiently generic solution would require adding specifications to the geometry and some logic, which I don't think is worth given that these numbers are not expected to change (last change was 8 years ago and none is in sight in the future)

cmsbuild · 2020-04-24T12:23:29Z

The code-checks are being triggered in jenkins.

cmsbuild · 2020-04-24T12:32:04Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-29556/14849

This PR adds an extra 60KB to repository

Code check has found code style and quality issues which could be resolved by applying following patch(s)

code-checks:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-29556/14849/code-checks.patch
e.g. curl https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-29556/14849/code-checks.patch | patch -p1
You can also run scram build code-checks to apply code checks directly

slava77 · 2020-04-24T15:00:20Z

Code check has found code style and quality issues which could be resolved by applying following patch(s)

just in case it was missed

cmsbuild · 2020-04-24T15:17:15Z

The code-checks are being triggered in jenkins.

cmsbuild · 2020-04-24T15:24:26Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-29556/14854

This PR adds an extra 60KB to repository

cmsbuild · 2020-04-30T20:06:56Z

The code-checks are being triggered in jenkins.

cmsbuild · 2020-04-30T20:13:58Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-29556/14945

This PR adds an extra 124KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File MagneticField/Engine/python/volumeBasedMagneticField_160812_cfi.py modified in PR(s): Tentative implementation of TLS cache for MF #29561
- File MagneticField/Engine/python/volumeBasedMagneticField_71212_cfi.py modified in PR(s): Tentative implementation of TLS cache for MF #29561
- File MagneticField/Engine/test/MakePlots.sh modified in PR(s): Tentative implementation of TLS cache for MF #29561
- File MagneticField/Engine/test/README modified in PR(s): Tentative implementation of TLS cache for MF #29561
- File MagneticField/Engine/test/plot3d.gnuplot modified in PR(s): Tentative implementation of TLS cache for MF #29561
- File MagneticField/Engine/test/regression.py modified in PR(s): Tentative implementation of TLS cache for MF #29561
- File MagneticField/Engine/test/testMagneticField.cc modified in PR(s): Tentative implementation of TLS cache for MF #29561
- File MagneticField/GeomBuilder/src/BaseVolumeHandle.cc modified in PR(s): Tentative implementation of TLS cache for MF #29561
- File MagneticField/GeomBuilder/src/DD4hep_MagGeoBuilder.cc modified in PR(s): Tentative implementation of TLS cache for MF #29561
- File MagneticField/GeomBuilder/src/DD4hep_volumeHandle.cc modified in PR(s): Tentative implementation of TLS cache for MF #29561
- File MagneticField/GeomBuilder/src/MagGeoBuilderFromDDD.cc modified in PR(s): Tentative implementation of TLS cache for MF #29561
- File MagneticField/GeomBuilder/src/bLayer.cc modified in PR(s): Tentative implementation of TLS cache for MF #29561
- File MagneticField/GeomBuilder/src/bRod.cc modified in PR(s): Tentative implementation of TLS cache for MF #29561
- File MagneticField/GeomBuilder/src/bSector.cc modified in PR(s): Tentative implementation of TLS cache for MF #29561
- File MagneticField/GeomBuilder/src/bSlab.cc modified in PR(s): Tentative implementation of TLS cache for MF #29561
- File MagneticField/GeomBuilder/src/bSlab.h modified in PR(s): Tentative implementation of TLS cache for MF #29561
- File MagneticField/GeomBuilder/src/eSector.cc modified in PR(s): Tentative implementation of TLS cache for MF #29561
- File MagneticField/GeomBuilder/src/volumeHandle.cc modified in PR(s): Tentative implementation of TLS cache for MF #29561
- File MagneticField/GeomBuilder/test/python/testMagGeometry.py modified in PR(s): Tentative implementation of TLS cache for MF #29561
- File MagneticField/GeomBuilder/test/python/testMagGeometryOldDD.py modified in PR(s): Tentative implementation of TLS cache for MF #29561
- File MagneticField/GeomBuilder/test/stubs/MagGeometryAnalyzer.cc modified in PR(s): Tentative implementation of TLS cache for MF #29561
- File MagneticField/Layers/interface/MagVerbosity.h modified in PR(s): Tentative implementation of TLS cache for MF #29561
- File MagneticField/Layers/src/MagBLayer.cc modified in PR(s): Tentative implementation of TLS cache for MF #29561
- File MagneticField/Layers/src/MagBRod.cc modified in PR(s): Tentative implementation of TLS cache for MF #29561
- File MagneticField/Layers/src/MagBSector.cc modified in PR(s): Tentative implementation of TLS cache for MF #29561
- File MagneticField/Layers/src/MagBSlab.cc modified in PR(s): Tentative implementation of TLS cache for MF #29561
- File MagneticField/Layers/src/MagELayer.cc modified in PR(s): Tentative implementation of TLS cache for MF #29561
- File MagneticField/Layers/src/MagESector.cc modified in PR(s): Tentative implementation of TLS cache for MF #29561
- File MagneticField/VolumeBasedEngine/interface/MagGeometry.h modified in PR(s): Tentative implementation of TLS cache for MF #29561
- File MagneticField/VolumeBasedEngine/interface/VolumeBasedMagneticField.h modified in PR(s): Tentative implementation of TLS cache for MF #29561
- File MagneticField/VolumeBasedEngine/src/MagGeometry.cc modified in PR(s): Tentative implementation of TLS cache for MF #29561
- File MagneticField/VolumeBasedEngine/src/VolumeBasedMagneticField.cc modified in PR(s): Tentative implementation of TLS cache for MF #29561

cmsbuild · 2020-04-30T20:14:21Z

Pull request #29556 was updated. @perrotta, @cmsbuild, @slava77 can you please check and sign again.

slava77 · 2020-04-30T20:55:35Z

@cmsbuild please test

cmsbuild · 2020-04-30T20:56:00Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/5938/console Started: 2020/04/30 23:05

cmsbuild · 2020-04-30T22:18:13Z

+1
Tested at: 8850003
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-c491c8/5938/summary.html
CMSSW: CMSSW_11_1_X_2020-04-30-1100
SCRAM_ARCH: slc7_amd64_gcc820

cmsbuild · 2020-04-30T22:18:15Z

Comparison job queued.

cmsbuild · 2020-04-30T23:49:04Z

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-c491c8/5938/summary.html

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 34
DQMHistoTests: Total histograms compared: 2696239
DQMHistoTests: Total failures: 1
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2695919
DQMHistoTests: Total skipped: 319
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 33 files compared)
Checked 147 log files, 16 edm output root files, 34 DQM output files

slava77 · 2020-05-01T00:19:07Z

+1

for #29556 8850003

code changes are in line with the PR description and the follow up review
jenkins tests pass and comparisons with the baseline show no differences

cmsbuild · 2020-05-01T00:19:33Z

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

silviodonato · 2020-05-01T08:53:05Z

+1

namapane added 3 commits April 23, 2020 17:40

Remove unused cfi flag

7f9856c

Minor optmization proposed by @amadio: compute barrel dimensions at c…

9db8f5f

…onstruction time. Improves 0.4% CPU for non-cached calls. Also migrate debug couts to MessageLogger

More minor optimization and cleanup

5bf6985

cmsbuild added this to the CMSSW_11_1_X milestone Apr 24, 2020

cmsbuild added code-checks-pending comparison-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Apr 24, 2020

namapane mentioned this pull request Apr 24, 2020

Fix a scaling issue in MagneticField/VolumeBasedEngine #28180

Closed

cmsbuild added code-checks-rejected and removed code-checks-pending labels Apr 24, 2020

slava77 reviewed Apr 24, 2020

View reviewed changes

code-checks

852b82e

cmsbuild added code-checks-pending and removed code-checks-rejected labels Apr 24, 2020

cmsbuild added code-checks-rejected and removed code-checks-pending labels Apr 24, 2020

code-checks

b5639f5

cmsbuild added code-checks-pending and removed code-checks-rejected labels Apr 24, 2020

cmsbuild removed the code-checks-pending label Apr 24, 2020

cmsbuild added code-checks-pending comparison-pending tests-pending labels Apr 30, 2020

cmsbuild added code-checks-approved and removed code-checks-pending labels Apr 30, 2020

cmsbuild added tests-started and removed tests-pending labels Apr 30, 2020

cmsbuild added tests-approved and removed tests-started labels Apr 30, 2020

cmsbuild added comparison-available and removed comparison-pending labels Apr 30, 2020

cmsbuild added fully-signed reconstruction-approved and removed pending-signatures reconstruction-pending labels May 1, 2020

silviodonato changed the title ~~Mf optimization~~ Magnetic Field optimization May 1, 2020

cmsbuild added orp-approved and removed orp-pending labels May 1, 2020

cmsbuild merged commit ae6f5ca into cms-sw:master May 1, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Magnetic Field optimization #29556

Magnetic Field optimization #29556

namapane commented Apr 24, 2020

cmsbuild commented Apr 24, 2020

cmsbuild commented Apr 24, 2020

slava77 Apr 24, 2020

namapane Apr 24, 2020

cmsbuild commented Apr 24, 2020

cmsbuild commented Apr 24, 2020

slava77 commented Apr 24, 2020

cmsbuild commented Apr 24, 2020

cmsbuild commented Apr 24, 2020

cmsbuild commented Apr 30, 2020

cmsbuild commented Apr 30, 2020

cmsbuild commented Apr 30, 2020

slava77 commented Apr 30, 2020

cmsbuild commented Apr 30, 2020 •

edited

cmsbuild commented Apr 30, 2020

cmsbuild commented Apr 30, 2020

cmsbuild commented Apr 30, 2020

slava77 commented May 1, 2020

cmsbuild commented May 1, 2020

silviodonato commented May 1, 2020

Magnetic Field optimization #29556

Magnetic Field optimization #29556

Conversation

namapane commented Apr 24, 2020

PR description:

PR validation:

cmsbuild commented Apr 24, 2020

cmsbuild commented Apr 24, 2020

slava77 Apr 24, 2020

Choose a reason for hiding this comment

namapane Apr 24, 2020

Choose a reason for hiding this comment

cmsbuild commented Apr 24, 2020

cmsbuild commented Apr 24, 2020

slava77 commented Apr 24, 2020

cmsbuild commented Apr 24, 2020

cmsbuild commented Apr 24, 2020

cmsbuild commented Apr 30, 2020

cmsbuild commented Apr 30, 2020

cmsbuild commented Apr 30, 2020

slava77 commented Apr 30, 2020

cmsbuild commented Apr 30, 2020 • edited

cmsbuild commented Apr 30, 2020

cmsbuild commented Apr 30, 2020

cmsbuild commented Apr 30, 2020

slava77 commented May 1, 2020

cmsbuild commented May 1, 2020

silviodonato commented May 1, 2020

cmsbuild commented Apr 30, 2020 •

edited