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.

Sign up for GitHub

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

Jump to bottom

Run3-gex108XB Backport SimCore/SimG4CMS related files which can work with the new Run3 geometry #36780

Merged

cmsbuild merged 2 commits into cms-sw:CMSSW_12_2_X from bsunanda:Run3-gex108XB

Feb 7, 2022

Contributor

bsunanda commented Jan 22, 2022 •

edited by perrotta

Loading

PR description:

Backport SimCore/SimG4CMS related files which can work with the new Run3 geometry

PR validation:

Use the runTheMatrix test workflows

if this PR is a backport please specify the original PR and why you need to backport that PR:

Backporting the PR's #36652, #36680, #36750, #36411, #36524 which were used to introduce the new Run3 geometry

cmsbuild added this to the CMSSW_12_2_X milestone

cmsbuild added operations-pending orp-pending pending-signatures simulation-pending tests-pending labels

Contributor

cmsbuild commented Jan 22, 2022

A new Pull Request was created by @bsunanda (Sunanda Banerjee) for CMSSW_12_2_X.

It involves the following packages:

Configuration/Eras (operations)
SimG4CMS/Forward (simulation)
SimG4CMS/Muon (simulation)
SimG4Core/Application (simulation)

@perrotta, @civanch, @mdhildreth, @cmsbuild, @qliphy, @fabiocos, @davidlange6 can you please review it and eventually sign? Thanks.
@makortel, @cvuosalo, @rovere, @Martin-Grunewald, @missirol, @fabiocos, @slomeo 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

Contributor Author

bsunanda commented Jan 22, 2022

@cmsbuild Please test

cmsbuild added tests-started and removed tests-pending labels

Contributor

cmsbuild commented Jan 22, 2022

-1

Failed Tests: RelVals-INPUT
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-135f05/21925/summary.html
COMMIT: 24f0ae9
CMSSW: CMSSW_12_2_X_2022-01-22-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/36780/21925/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals-INPUT

4.764.76_ZMuSkim2012D+ZMuSkim2012D+HLTDSKIM2+RECODR1reHLT2+HARVESTDR1reHLT/step2_ZMuSkim2012D+ZMuSkim2012D+HLTDSKIM2+RECODR1reHLT2+HARVESTDR1reHLT.log

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 2 differences found in the comparisons
DQMHistoTests: Total files compared: 42
DQMHistoTests: Total histograms compared: 3250594
DQMHistoTests: Total failures: 6
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3250566
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 41 files compared)
Checked 177 log files, 37 edm output root files, 42 DQM output files
TriggerResults: no differences found

cmsbuild added tests-rejected and removed tests-started labels

bsunanda mentioned this pull request

Run3-gex108XD Modify muon numbering, adding scenarios for the corretced Run3 geometry in CMSSW version 12_2_X #36782

Merged

francescobrivio mentioned this pull request

Update final Run3 MC geometry [12_2_X] #36785

Merged

Contributor

civanch commented Jan 23, 2022

+1

cmsbuild added simulation-approved and removed simulation-pending labels

Contributor

perrotta commented Jan 23, 2022

Backport of #36750, #36652
(please check, and update the PR description with it)

perrotta reviewed

View reviewed changes

Configuration/Eras/python/Modifier_run2_ECAL_2018_cff.py Outdated

		@@ -0,0 +1,3 @@
		import FWCore.ParameterSet.Config as cms

		run2_ECAL_2018 = cms.Modifier()

Contributor

perrotta Jan 27, 2022

This modifier does not seem to be needed: please remove

Configuration/Eras/python/Modifier_run3_geomOld_cff.py Outdated

		@@ -0,0 +1,4 @@
		import FWCore.ParameterSet.Config as cms

		run3_geomOld = cms.Modifier()

Contributor

perrotta Jan 27, 2022

This modifier must be backported with #36782, where it is missing, instead

SimG4Core/Application/python/g4SimHits_cfi.py Outdated

@@ @@ -86,6 +86,7 @@ @@
                   RestorePhysicsTables = cms.untracked.bool(False),
                   UseParametrisedEMPhysics = cms.untracked.bool(True),
                   ThresholdForGeometryExceptions = cms.double(0.01),   ## in GeV
+                  TraceExceptions = cms.bool(False),

Contributor

perrotta Jan 27, 2022

Where does it come from?
Why is it needed?

SimG4Core/Application/python/g4SimHits_cfi.py

               ##
               from Configuration.Eras.Modifier_run3_common_cff import run3_common
               run3_common.toModify( g4SimHits, CastorSD = dict( useShowerLibrary = False ) )
+              run3_common.toModify( g4SimHits, LHCTransport = False )

Contributor

perrotta Jan 27, 2022 •

edited

Loading

This comes from #36524: could you please clean up from unwanted commits?

SimG4Core/Application/python/g4SimHits_cfi.py

-              phase2_timing.toModify( g4SimHits.ECalSD,
-                                           StoreLayerTimeSim = cms.untracked.bool(True),
-                                           TimeSliceUnit = cms.double(0.001) )
+              phase2_timing.toModify( g4SimHits, ECalSD = dict(

Contributor

perrotta Jan 27, 2022 •

edited

Loading

This comes from #36524: could you please clean up from unwanted commits?

SimG4Core/Application/python/g4SimHits_cfi.py

+              ##
+              ## Disable PPS from Run 3 PbPb runs
+              ##
+              from Configuration.Eras.Modifier_pp_on_PbPb_run3_cff import pp_on_PbPb_run3

Contributor

perrotta Jan 27, 2022

This is from #36411: is it needed? I would say no, because LHCTransport is already disabled for Run3. It would deserve a comment, anyhow

SimG4Core/Application/python/g4SimHits_cfi.py

-              h2tb.toModify(g4SimHits.CaloSD,
-                            EminHits  = cms.vdouble(0.0,0.0,0.0,0.0,0.0),
-                            TmaxHits  = cms.vdouble(1000.0,1000.0,1000.0,1000.0,2000.0) )
+              h2tb.toModify(g4SimHits, CaloSD = dict(

Contributor

perrotta Jan 27, 2022 •

edited

Loading

Also from #36524

SimG4Core/Application/python/g4SimHits_cfi.py

@@ @@ -652,9 +666,13 @@ @@
               dd4hep.toModify( g4SimHits, g4GeometryDD4hepSource = True )
               ##
-              ## Selection of SD's for Phase2
+              ## Selection of SD's for Phase2, exclude PPS
               ##
               from Configuration.Eras.Modifier_phase2_common_cff import phase2_common
               phase2_common.toModify(g4SimHits,

Contributor

perrotta Jan 27, 2022

This is from #36411: is it needed? I would say no, because LHCTransport is already disabled for Run3. It would deserve a comment, anyhow

bsunanda pushed a commit to bsunanda/cmssw that referenced this pull request


          Add the modifier which was initially intended in cms-sw#36780

cmsbuild added simulation-pending tests-pending and removed operations-pending simulation-approved tests-rejected labels

Contributor

cmsbuild commented Jan 28, 2022

Pull request #36780 was updated. @cmsbuild, @civanch, @mdhildreth can you please check and sign again.

Contributor Author

bsunanda commented Jan 28, 2022

@perrotta I took care of all the comments. You may seee some corrections because I started with a build which was before merging #36524

Contributor Author

bsunanda commented Jan 28, 2022

@cmsbuild Please test

cmsbuild removed the tests-pending label

Contributor

perrotta commented Feb 3, 2022 •

edited

Loading

@bsunanda @civanch please agree about the appropriateness of backporting #36680, as it is done now (and possibly also the other exception related PRs as mentioned in #36780 (comment)). This is the only remaining issue with this backport PR, once #36780 (review) will also get addressed,

perrotta mentioned this pull request

[DDD] Fix building of DDD GEM reco geometry DB payload, backport of #36869 #36870

Merged

Contributor Author

bsunanda commented Feb 3, 2022

Without PR #36680 SD's of the forward detectors will not work for dd4hep. The forward detectors are now part of Run3 scenario and dd4hep is the default geometry base for Run3. So this PR #36680 must be back ported to12_2_X

Contributor

perrotta commented Feb 3, 2022

Without PR #36680 SD's of the forward detectors will not work for dd4hep. The forward detectors are now part of Run3 scenario and dd4hep is the default geometry base for Run3. So this PR #36680 must be back ported to12_2_X

Thank you Sunanda.
If @civanch also agree for me it's fine
Please cure #36780 (comment) anyhow

Contributor Author

bsunanda commented Feb 3, 2022

@perrotta Which fix to be cured - do you mean remove the line LHCTransport = False for phase2 modification?

Contributor

perrotta commented Feb 3, 2022

@perrotta Which fix to be cured - do you mean remove the line LHCTransport = False for phase2 modification?

No: to add it, as now it is missing

Contributor Author

bsunanda commented Feb 3, 2022

LHCTRansport = False is set with Run3Common. I am now doing it in Phase2 as well


          Add qualifier for Phase2

cmsbuild added tests-pending and removed tests-approved labels

Contributor

cmsbuild commented Feb 3, 2022

Pull request #36780 was updated. @cmsbuild, @civanch, @mdhildreth can you please check and sign again.

Contributor Author

bsunanda commented Feb 3, 2022

@cmsbuild Please test

cmsbuild added tests-started and removed tests-pending labels

Contributor Author

bsunanda commented Feb 3, 2022

@perrotta Only one thing I did in this update: added back the "LHCTransport = False" in g4SimHits_cfi.py

Contributor

perrotta commented Feb 3, 2022

@perrotta Only one thing I did in this update: added back the "LHCTransport = False" in g4SimHits_cfi.py

Thank you @bsunanda
Now the backport fully reflects what is merged in the master

Contributor

cmsbuild commented Feb 3, 2022

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-135f05/22196/summary.html
COMMIT: 8237336
CMSSW: CMSSW_12_2_X_2022-02-03-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/36780/22196/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 4 differences found in the comparisons
DQMHistoTests: Total files compared: 42
DQMHistoTests: Total histograms compared: 3250600
DQMHistoTests: Total failures: 5
DQMHistoTests: Total nulls: 1
DQMHistoTests: Total successes: 3250572
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: -0.004 KiB( 41 files compared)
DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
Checked 177 log files, 37 edm output root files, 42 DQM output files
TriggerResults: no differences found

cmsbuild added tests-approved and removed tests-started labels

Contributor

civanch commented Feb 3, 2022

+1

cmsbuild added fully-signed simulation-approved and removed simulation-pending pending-signatures labels

Contributor

cmsbuild commented Feb 3, 2022

This pull request is fully signed and it will be integrated in one of the next CMSSW_12_2_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_12_3_X is complete. 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)

Contributor

perrotta commented Feb 7, 2022

+1

cmsbuild added orp-approved and removed orp-pending labels

cmsbuild merged commit 18ca25b into cms-sw:CMSSW_12_2_X

perrotta mentioned this pull request

backport extended exception for geant4/vecgeom tracking #37381

Merged

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

Labels

fully-signed orp-approved simulation-approved tests-approved