Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[G4VECGEOM] Update clhep to 2.4.6.0, vecgeom to 1.2.1 #8211

Merged
merged 6 commits into from
Dec 7, 2022

Conversation

iarspider
Copy link
Contributor

Closes #8209, #8210.

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @iarspider for branch IB/CMSSW_12_6_X/g11_g4_vecgeom.

@cmsbuild, @smuzaffar, @aandvalenzuela, @iarspider can you please review it and eventually sign? Thanks.
@perrotta, @dpiparo, @rappoccio you are the release manager for this.
cms-bot commands are listed here

@iarspider iarspider changed the title Update clhep to 2.4.6.0, vecgeom to 1.2.1 [G4VECGEOM] Update clhep to 2.4.6.0, vecgeom to 1.2.1 Nov 30, 2022
@iarspider iarspider self-assigned this Nov 30, 2022
@smuzaffar
Copy link
Contributor

@iarspider , base branch for this PR is not correct

@iarspider iarspider changed the base branch from IB/CMSSW_12_6_X/g11_g4_vecgeom to IB/CMSSW_13_0_X/g11_g4_vecgeom December 1, 2022 14:23
@civanch
Copy link
Contributor

civanch commented Dec 2, 2022

Is base IB OK?

@smuzaffar
Copy link
Contributor

test parameters:

  • full_cmssw = true

@smuzaffar
Copy link
Contributor

G4 Vengeom IB is bit old , let me re-build a new one which we can use for testing

@smuzaffar
Copy link
Contributor

@civanch , looks like tests are failing due to vecgeom update here

#5  0x00002adf1c01e5a5 in vecgeom::cxx::CommonUnplacedVolumeImplHelper<vecgeom::cxx::PolyhedronImplementation<(EInnerRadii)0, (EPhiCutout)0>, vecgeom::cxx::VUnplacedVolume>::SafetyToIn(vecgeom::cxx::Vector3D<double> const&) const () from /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmsdist/8211/29458/CMSSW_13_0_G4VECGEOM_X_2022-12-04-2300/biglib/el8_amd64_gcc11/pluginSimulation.so
#6  0x00002adf1c021dc7 in G4UAdapter<vecgeom::cxx::UnplacedPolyhedron>::DistanceToIn(CLHEP::Hep3Vector const&) const () from /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmsdist/8211/29458/CMSSW_13_0_G4VECGEOM_X_2022-12-04-2300/biglib/el8_amd64_gcc11/pluginSimulation.so
#7  0x00002adf1c0e6274 in G4VoxelNavigation::ComputeStep(CLHEP::Hep3Vector const&, CLHEP::Hep3Vector const&, double, double&, G4NavigationHistory&, bool&, CLHEP::Hep3Vector&, bool&, bool&, G4VPhysicalVolume**, int&) () from /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmsdist/8211/29458/CMSSW_13_0_G4VECGEOM_X_2022-12-04-2300/biglib/el8_amd64_gcc11/pluginSimulation.so
#8  0x00002adf1bd00001 in G4Navigator::ComputeStep(CLHEP::Hep3Vector const&, CLHEP::Hep3Vector const&, double, double&) () from /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmsdist/8211/29458/CMSSW_13_0_G4VECGEOM_X_2022-12-04-2300/biglib/el8_amd64_gcc11/pluginSimulation.so
#9  0x00002adf1bf1733a in G4Transportation::AlongStepGetPhysicalInteractionLength(G4Track const&, double, double, double&, G4GPILSelection*) () from /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmsdist/8211/29458/CMSSW_13_0_G4VECGEOM_X_2022-12-04-2300/biglib/el8_amd64_gcc11/pluginSimulation.so
#10 0x00002adf1c554bf2 in G4SteppingManager::DefinePhysicalStepLength() () from /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmsdist/8211/29458/CMSSW_13_0_G4VECGEOM_X_2022-12-04-2300/biglib/el8_amd64_gcc11/pluginSimulation.so
#11 0x00002adf1c555d00 in G4SteppingManager::Stepping() () from /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmsdist/8211/29458/CMSSW_13_0_G4VECGEOM_X_2022-12-04-2300/biglib/el8_amd64_gcc11/pluginSimulation.so
#12 0x00002adf1bf0f27b in G4TrackingManager::ProcessOneTrack(G4Track*) () from /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmsdist/8211/29458/CMSSW_13_0_G4VECGEOM_X_2022-12-04-2300/biglib/el8_amd64_gcc11/pluginSimulation.so
#13 0x00002adf1bba0f79 in G4EventManager::DoProcessing(G4Event*) () from /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmsdist/8211/29458/CMSSW_13_0_G4VECGEOM_X_2022-12-04-2300/biglib/el8_amd64_gcc11/pluginSimulation.so
#14 0x00002adf1b80ae60 in RunManagerMTWorker::produce(edm::Event const&, edm::EventSetup const&, RunManagerMT&) () from /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmsdist/8211/29458/CMSSW_13_0_G4VECGEOM_X_2022-12-04-2300/biglib/el8_amd64_gcc11/pluginSimulation.so

@smuzaffar smuzaffar changed the base branch from IB/CMSSW_13_0_X/g11_g4_vecgeom to IB/CMSSW_13_0_X/g4_vecgeom December 5, 2022 13:48
@civanch
Copy link
Contributor

civanch commented Dec 5, 2022

@smuzaffar , I make a ticket to VecGeom and Jonas just report that -O2 with gcc11 resolve the issue. So, it is a very specific VecGeom problem (code seems to be correct but during optimisation non-initialized vector is crashing). Until it s understood -O2 should be used for VecGeom.

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 6, 2022

Pull request #8211 was updated.

@smuzaffar
Copy link
Contributor

please test
ok, I have updated dd4hep, vecgeom and geant4 to use -O2 -DNDEBUG

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 6, 2022

-1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-eac510/29481/summary.html
COMMIT: 2049e79
CMSSW: CMSSW_13_0_G4VECGEOM_X_2022-12-04-2300/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmsdist/8211/29481/install.sh to create a dev area with all the needed externals and cmssw changes.

External Build

I found compilation error when building:

+ '[' -f /data/cmsbld/jenkins/workspace/ib-run-pr-tests/testBuildDir/el8_amd64_gcc11/./etc/profile.d/init.sh ']'
+ export BOOST_ROOT
+ CMAKE_ARGS='-DCMAKE_INSTALL_PREFIX='\''/data/cmsbld/jenkins/workspace/ib-run-pr-tests/testBuildDir/tmp/BUILDROOT/311365d3eaa0f99a8fdd92609ab78336/opt/cmssw/el8_amd64_gcc11/external/dd4hep/v01-23x-311365d3eaa0f99a8fdd92609ab78336'\''       -DBoost_NO_BOOST_CMAKE=ON       -DDD4HEP_USE_XERCESC=ON       -DXERCESC_ROOT_DIR=/data/cmsbld/jenkins/workspace/ib-run-pr-tests/testBuildDir/el8_amd64_gcc11/external/xerces-c/3.1.3-96261f23c7d6fbfb7d59be544bd882f3       -DDD4HEP_USE_PYROOT=ON       -DCMAKE_CXX_STANDARD=17       -DCMAKE_BUILD_TYPE=Release       -DCMAKE_CXX_FLAGS_RELEASE=-O2'
+ '-DNDEBUG       -DDD4HEP_USE_GEANT4_UNITS=ON       -DCMAKE_PREFIX_PATH=/data/cmsbld/jenkins/workspace/ib-run-pr-tests/testBuildDir/el8_amd64_gcc11/external/clhep/2.4.6.0-80baf510802496a74335c6030dc0c72b;/data/cmsbld/jenkins/workspace/ib-run-pr-tests/testBuildDir/el8_amd64_gcc11/external/xerces-c/3.1.3-96261f23c7d6fbfb7d59be544bd882f3'
/data/cmsbld/jenkins/workspace/ib-run-pr-tests/testBuildDir/tmp/rpm-tmp.C4cLqv: line 40: -DNDEBUG       -DDD4HEP_USE_GEANT4_UNITS=ON       -DCMAKE_PREFIX_PATH=/data/cmsbld/jenkins/workspace/ib-run-pr-tests/testBuildDir/el8_amd64_gcc11/external/clhep/2.4.6.0-80baf510802496a74335c6030dc0c72b;/data/cmsbld/jenkins/workspace/ib-run-pr-tests/testBuildDir/el8_amd64_gcc11/external/xerces-c/3.1.3-96261f23c7d6fbfb7d59be544bd882f3: No such file or directory
error: Bad exit status from /data/cmsbld/jenkins/workspace/ib-run-pr-tests/testBuildDir/tmp/rpm-tmp.C4cLqv (%build)


RPM build errors:
line 37: It's not recommended to have unversioned Obsoletes: Obsoletes: external+dd4hep+v01-23x-311365d3eaa0f99a8fdd92609ab78336
Bad exit status from /data/cmsbld/jenkins/workspace/ib-run-pr-tests/testBuildDir/tmp/rpm-tmp.C4cLqv (%build)


@smuzaffar
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 6, 2022

Pull request #8211 was updated.

@aandvalenzuela
Copy link
Contributor

aandvalenzuela commented Dec 7, 2022

It seems comparison is not going to report the results back since it is failing at:

09:08:17 + mv /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-eac510/1000.0_RunMinBias2011A /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-eac510/1000.0_RunMinBias2011A
09:08:17 mv: cannot move '/data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-eac510/1000.0_RunMinBias2011A' to a subdirectory of itself, '/data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-eac510/1000.0_RunMinBias2011A/1000.0_RunMinBias2011A'

I am writing a small fix for this

@smuzaffar
Copy link
Contributor

thanks @aandvalenzuela for checking this. cms-sw/cms-bot@25334a8 change should fix the issue. I have restarted the comparison job now

@aandvalenzuela
Copy link
Contributor

I first added this extra check https://github.com/cms-sw/cms-bot/blob/master/pr_testing/run-pr-comparisons#L126, should I keep it @smuzaffar?

@smuzaffar
Copy link
Contributor

thanks for typo fix at cms-sw/cms-bot@06cf850 :-)
No need to have https://github.com/cms-sw/cms-bot/blob/master/pr_testing/run-pr-comparisons#L126 but it is harmless, so no need to remove it

@hahnjo
Copy link
Contributor

hahnjo commented Dec 7, 2022

FWIW I opened https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108008, let's see what the GCC people think

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 7, 2022

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-eac510/29485/summary.html
COMMIT: 7efc76b
CMSSW: CMSSW_13_0_G4VECGEOM_X_2022-12-04-2300/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmsdist/8211/29485/install.sh to create a dev area with all the needed externals and cmssw changes.

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:

You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-eac510/29485/git-recent-commits.json
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-eac510/29485/git-merge-result

Comparison Summary

Summary:

  • No significant changes to the logs found
  • ROOTFileChecks: Some differences in event products or their sizes found
  • Reco comparison results: 62397 differences found in the comparisons
  • DQMHistoTests: Total files compared: 48
  • DQMHistoTests: Total histograms compared: 3421214
  • DQMHistoTests: Total failures: 116323
  • DQMHistoTests: Total nulls: 247
  • DQMHistoTests: Total successes: 3304622
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: -11.074000000000002 KiB( 47 files compared)
  • DQMHistoSizes: changed ( 10224.0 ): 0.469 KiB SiStrip/MechanicalView
  • DQMHistoSizes: changed ( 11634.0,... ): -0.774 KiB Physics/NanoAODDQM
  • DQMHistoSizes: changed ( 11834.0 ): 0.663 KiB SiStrip/MechanicalView
  • DQMHistoSizes: changed ( 13234.0,... ): -0.454 KiB Physics/NanoAODDQM
  • DQMHistoSizes: changed ( 250202.181 ): -0.053 KiB SiStrip/MechanicalView
  • DQMHistoSizes: changed ( 25202.0 ): -0.416 KiB SiStrip/MechanicalView
  • DQMHistoSizes: changed ( 7.3 ): -6.948 KiB SiStrip/MechanicalView
  • DQMHistoSizes: changed ( 8.0 ): 1.537 KiB SiStrip/MechanicalView
  • Checked 206 log files, 158 edm output root files, 48 DQM output files
  • TriggerResults: found differences in 15 / 46 workflows

@smuzaffar
Copy link
Contributor

@civanch , this looks good now. I am merging it for next G4 Vecgeom IB

@smuzaffar smuzaffar merged commit 7019863 into IB/CMSSW_13_0_X/g4_vecgeom Dec 7, 2022
@civanch
Copy link
Contributor

civanch commented Dec 7, 2022

@smuzaffar , thanks, it is confirmed by gcc that there is problem in -O3. When new IB will be available will try to check CPU.

@smuzaffar
Copy link
Contributor

@civanch , CMSSW_13_0_G4VECGEOM_X_2022-12-07-2300 and CMSSW_13_0_GEANT4_X_2022-12-07-2300 IBs are available

@civanch
Copy link
Contributor

civanch commented Dec 9, 2022

I have check CPU. It turned out that this switch from -O3 to -O2 provide CPU penalty 2.2% for MinBias and 4.9% for ttbar. So, we need -O3 indeed. The solution may be work around for problematic class of VecGeom

@hahnjo
Copy link
Contributor

hahnjo commented Dec 9, 2022

Yes, I can also measure the performance penalty. We may try working around the problem (I can prepare a patch), or we can try compiling Geant4 with -O3 and only VecGeom with -O2 (as we did in the past).

@smuzaffar smuzaffar deleted the update-clhep-vecgeom branch December 9, 2022 22:36
@hahnjo
Copy link
Contributor

hahnjo commented Jan 10, 2023

FWIW the bug in GCC should be fixed, cf. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107323 and https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108008. Once there is an updated compiler with the fixes, we can consider going back to -O3 also for VecGeom.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migration of G4VECGEOM branch to VecGeom 1.2.1 Migration to CLHEP 2.4.6.0
6 participants