Add LHCTransport to Run3 GEN content #37470

srimanob · 2022-04-05T16:09:28Z

PR description:

As reported in #37469
We can't split GEN and SIM for Run-3 because of missing
edm::HepMCProduct "LHCTransport" "" "GEN"

This PR adds it as default collection of GENRAW and GENRECO.

PR validation:

Test with 2 steps, GEN and SIM, separately wotks:
cmsDriver.py TTbar_14TeV_TuneCP5_cfi -s GEN,SIM -n 10 --conditions auto:phase1_2021_realistic --beamspot Run3RoundOptics25ns13TeVLowSigmaZ --datatier GEN-SIM --eventcontent RAWSIM --geometry DB:Extended --era Run3 --python step1_TTbar_14TeV_GEN.py --no_exec --fileout file:step1.root --nThreads 8

cmsDriver.py step2 -s SIM --conditions auto:phase1_2021_realistic --datatier GEN-SIM -n -1 --eventcontent FEVTDEBUGHLT --geometry DB:Extended --era Run3 --python step2_SIM_2021.py --no_exec --filein file:step1.root --fileout file:step2.root --nThreads 8

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

No need of backport if we don't need to split them in the release before 12_4.

cmsbuild · 2022-04-05T16:16:54Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37470/29164

This PR adds an extra 12KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File GeneratorInterface/Configuration/python/GeneratorInterface_EventContent_cff.py modified in PR(s): Refactoring gen weight storage in EDM + Nano integration #32167

cmsbuild · 2022-04-05T16:17:13Z

A new Pull Request was created by @srimanob (Phat Srimanobhas) for master.

It involves the following packages:

GeneratorInterface/Configuration (generators)

@SiewYan, @mkirsano, @Saptaparna, @cmsbuild, @GurpreetSinghChahal, @alberto-sanchez can you please review it and eventually sign? Thanks.
@alberto-sanchez, @mkirsano 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

davidlange6 · 2022-04-05T16:19:39Z

what is the cost of just adding this to all gen (reducing complexity)?

makortel · 2022-04-05T16:33:28Z

GeneratorInterface/Configuration/python/GeneratorInterface_EventContent_cff.py

+
+from Configuration.Eras.Modifier_run3_common_cff import run3_common
+run3_common.toModify(GeneratorInterfaceRAW,
+                     outputCommands = GeneratorInterfaceRAW.outputCommands+['keep edmHepMCProduct_LHCTransport_*_*'])


If I've understood correctly, the LHCTransport module is enabled to be run here

cmssw/SimPPS/Configuration/python/GenPPS_cff.py

Lines 18 to 19 in cb97260

from Configuration.Eras.Modifier_ctpps_2021_cff import ctpps_2021

ctpps_2021.toReplaceWith(PPSTransportTask, cms.Task(LHCTransport))

It would be better to use the more specific modifiers. I see the run3_common being used also here

cmssw/SimG4Core/Application/python/g4SimHits_cfi.py

Line 636 in cb97260

run3_common.toModify( g4SimHits, LHCTransport = True )

If that would be changed to use ctpps_2021 as well, and the definition of Run3_pp_on_PbPb

cmssw/Configuration/Eras/python/Era_Run3_pp_on_PbPb_cff.py

Line 9 in cb97260

Run3_pp_on_PbPb = cms.ModifierChain(Run3_noMkFit.copyAndExclude([trackdnn, trackdnn_CKF, trackingNoLoopers]), pp_on_AA, pp_on_PbPb_run3)

to exclude ctpps_2021 (assuming that is the intention), then this fragment

cmssw/SimG4Core/Application/python/g4SimHits_cfi.py

Lines 638 to 642 in cb97260

##

## Disable PPS from Run 3 PbPb runs

##

from Configuration.Eras.Modifier_pp_on_PbPb_run3_cff import pp_on_PbPb_run3

pp_on_PbPb_run3.toModify( g4SimHits, LHCTransport = False )

would not be needed, nor any explicit treatment for Phase2 (since ctpps_2021 is excluded from there

cmssw/Configuration/Eras/python/Era_Phase2_cff.py

Line 21 in cb97260

Phase2 = cms.ModifierChain(Run3_noMkFit.copyAndExclude([phase1Pixel,trackingPhase1,ctpps_2021,dd4hep]), phase2_common, phase2_tracker, trackingPhase2PU140, phase2_ecal, phase2_hcal, phase2_hgcal, phase2_muon, phase2_GEM, hcalHardcodeConditions, phase2_timing, phase2_timing_layer, phase2_trigger)

).

srimanob · 2022-04-05T16:47:31Z

Hi @davidlange6 @makortel

Thanks for prompt comment. The added collection is very cheap,
Branch Name | Average Uncompressed Size (Bytes/Event) | Average Compressed Size (Bytes/Event)
edmHepMCProduct_generatorSmeared__GEN. 374795 103862
edmHepMCProduct_LHCTransport__GEN. 442.5 268.7 <<=== What is added in this PR.

If OK, I can put it in the GEN collection by default.
And I will recheck and update modifiers in other places (maybe in the separated PR).

FYI @civanch

civanch · 2022-04-06T08:33:42Z

@srimanob , LHCTransport return list of particles after Hector transport in beam pipe. This allows making simulation of interaction of protons with PPS detectors in Geant4. We add these protons after Hector transport to the list of primary for Geant4. So, Hector transport plugin should run before OscarProducer. It may have from 0 to 2 protons only, so the size is negligible.

If it is unavoidable to do this transport at GEN step it is fine what you propose. However, logically I would make Hector transport within SIM step not GEN step.

civanch · 2022-04-06T08:40:07Z

I would say more: Hector transport is usually known after the experiment not before, so having Hector transport inside GEN may be a problem.

srimanob · 2022-04-06T08:52:25Z

Hi @civanch
Thanks for the comment, but I'm not sure I understand correctly.

When you say "so having Hector transport inside GEN may be a problem.", do you mean it is not done properly now (before this PR)? Since this PR does not touch what currently happen in GEN and SIM, so I understand that what to be fixed is moving the LHCTransport out from GEN.

My goal is clear, I would like to make sure we can decouple GEN and SIM step. This is how it runs now, at the end of GEN step before entering SIM.
https://github.com/cms-sw/cmssw/blob/master/Configuration/StandardSequences/python/Generator_cff.py#L59
pgen = cms.Sequence(cms.SequencePlaceholder("randomEngineStateProducer")+VertexSmearing+GenSmeared+GeneInfo+genJetMET, PPSTransportTask)

civanch · 2022-04-06T09:00:08Z

@srimanob , properties of beam optics are taken from Run-2 and during Run-3 some modifications may be required (or not). It is a typical problem of PPS, may be after first few beams these parameters may be tuned as the beam spot, we need to communicate PPS peoples to clarify that. It is why my suggesting to have the Hector transport at the SIM step is not strong: if it is easy, better to do it at the SIM step, because Hector is a kind of simulation. If it is too complicate now - let us left Hector at the GEN step.

srimanob · 2022-04-06T09:41:51Z

OK, so I fix this PR by make it by default as @davidlange6 suggests.
And I will open another PR to review the use of customization as suggested by @makortel

cmsbuild · 2022-04-06T09:44:57Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37470/29176

This PR adds an extra 12KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File GeneratorInterface/Configuration/python/GeneratorInterface_EventContent_cff.py modified in PR(s): Refactoring gen weight storage in EDM + Nano integration #32167

cmsbuild · 2022-04-06T09:45:16Z

Pull request #37470 was updated. @SiewYan, @mkirsano, @Saptaparna, @cmsbuild, @GurpreetSinghChahal, @alberto-sanchez can you please check and sign again.

srimanob · 2022-04-06T09:53:47Z

@cmsbuild please test

cmsbuild · 2022-04-06T13:45:14Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-5e7f5c/23682/summary.html
COMMIT: d7f99e7
CMSSW: CMSSW_12_4_X_2022-04-05-2300/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/37470/23682/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
ROOTFileChecks: Some differences in event products or their sizes found
Reco comparison results: 4 differences found in the comparisons
DQMHistoTests: Total files compared: 48
DQMHistoTests: Total histograms compared: 3593039
DQMHistoTests: Total failures: 8
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3593009
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
Checked 200 log files, 45 edm output root files, 48 DQM output files
TriggerResults: no differences found

civanch · 2022-04-09T10:24:14Z

@GurpreetSinghChahal, @SiewYan, @mkirsano, @Saptaparna, @alberto-sanchez, please, consider this PR, comment or sign.

It is needed to have step0 - GEN only. This is useful for many tests, because allows reusing the same sample of primary generated events in different runs, let say, FastSim/FullSim. The problem for Run-3 is that for PPS sub-detector simulation it is needed making Hector transport of very forward protons in magnetic system of LHC from IR to PPS stations at ~200 m away before doing Geant4 simulation. This update should not affect any WF result.

SiewYan · 2022-04-11T02:42:50Z

+1

I am fine with the changes. Changes suggested by @makortel will be implemented on different PR by @srimanob .

cmsbuild · 2022-04-11T02:43:14Z

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

srimanob · 2022-04-11T02:54:01Z

Thanks @SiewYan
Yes, update on modifier will go with the next PR (to review modifier of ctpps)

qliphy · 2022-04-12T00:52:14Z

+1

Run3 customization for GEN content

2b7d9e8

srimanob mentioned this pull request Apr 5, 2022

GEN and SIM cannot split in Run-3 due to missing edmHepMCProduct_LHCTransport output from GEN #37469

Closed

cmsbuild added this to the CMSSW_12_4_X milestone Apr 5, 2022

cmsbuild added code-checks-pending generators-pending orp-pending pending-signatures tests-pending labels Apr 5, 2022

cmsbuild added code-checks-approved and removed code-checks-pending labels Apr 5, 2022

makortel reviewed Apr 5, 2022

View reviewed changes

add LHCTransport by default

d7f99e7

cmsbuild added code-checks-pending and removed code-checks-approved labels Apr 6, 2022

cmsbuild added code-checks-approved and removed code-checks-pending labels Apr 6, 2022

cmsbuild added tests-started and removed tests-pending labels Apr 6, 2022

cmsbuild added tests-approved and removed tests-started labels Apr 6, 2022

cmsbuild added fully-signed generators-approved and removed generators-pending pending-signatures labels Apr 11, 2022

cmsbuild added orp-approved and removed orp-pending labels Apr 12, 2022

cmsbuild merged commit 34e13fc into cms-sw:master Apr 12, 2022

This was referenced Apr 12, 2022

Add import test for professor2 #37535

Merged

Update vecgeom to 1.1.20 cms-sw/cmsdist#7735

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add LHCTransport to Run3 GEN content #37470

Add LHCTransport to Run3 GEN content #37470

srimanob commented Apr 5, 2022 •

edited

cmsbuild commented Apr 5, 2022

cmsbuild commented Apr 5, 2022

davidlange6 commented Apr 5, 2022

makortel Apr 5, 2022

srimanob commented Apr 5, 2022 •

edited

civanch commented Apr 6, 2022

civanch commented Apr 6, 2022

srimanob commented Apr 6, 2022 •

edited

civanch commented Apr 6, 2022

srimanob commented Apr 6, 2022

cmsbuild commented Apr 6, 2022

cmsbuild commented Apr 6, 2022

srimanob commented Apr 6, 2022

cmsbuild commented Apr 6, 2022

civanch commented Apr 9, 2022

SiewYan commented Apr 11, 2022

cmsbuild commented Apr 11, 2022

srimanob commented Apr 11, 2022

qliphy commented Apr 12, 2022

	from Configuration.Eras.Modifier_ctpps_2021_cff import ctpps_2021
	ctpps_2021.toReplaceWith(PPSTransportTask, cms.Task(LHCTransport))

	##
	## Disable PPS from Run 3 PbPb runs
	##
	from Configuration.Eras.Modifier_pp_on_PbPb_run3_cff import pp_on_PbPb_run3
	pp_on_PbPb_run3.toModify( g4SimHits, LHCTransport = False )

Add LHCTransport to Run3 GEN content #37470

Add LHCTransport to Run3 GEN content #37470

Conversation

srimanob commented Apr 5, 2022 • edited

PR description:

PR validation:

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

cmsbuild commented Apr 5, 2022

cmsbuild commented Apr 5, 2022

davidlange6 commented Apr 5, 2022

makortel Apr 5, 2022

Choose a reason for hiding this comment

srimanob commented Apr 5, 2022 • edited

civanch commented Apr 6, 2022

civanch commented Apr 6, 2022

srimanob commented Apr 6, 2022 • edited

civanch commented Apr 6, 2022

srimanob commented Apr 6, 2022

cmsbuild commented Apr 6, 2022

cmsbuild commented Apr 6, 2022

srimanob commented Apr 6, 2022

cmsbuild commented Apr 6, 2022

Comparison Summary

civanch commented Apr 9, 2022

SiewYan commented Apr 11, 2022

cmsbuild commented Apr 11, 2022

srimanob commented Apr 11, 2022

qliphy commented Apr 12, 2022

srimanob commented Apr 5, 2022 •

edited

srimanob commented Apr 5, 2022 •

edited

srimanob commented Apr 6, 2022 •

edited