Allow specification of ES modules in ExternalGeneratorFilter #34146

Dr15Jones · 2021-06-15T18:52:49Z

PR description:

The parameter external_process_esModules allows one to specify the names of ES modules which should be copied from the main process and started in the external process.

PR validation:

I locally copied the test config compare_external_generators_cfg.py and added the Tracer service via the parameter. The tracing showed up in the output from the external processes.

resolves cms-sw/framework-team#168

The parameter _external_process_esModules_ allows one to specify the names of ES modules which should be copied from the main process and started in the external process.

cmsbuild · 2021-06-15T18:59:22Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34146/23342

This PR adds an extra 16KB to repository

cmsbuild · 2021-06-15T18:59:46Z

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

It involves the following packages:

GeneratorInterface/Core

@SiewYan, @mkirsano, @cmsbuild, @GurpreetSinghChahal, @agrohsje, @alberto-sanchez can you please review it and eventually sign? Thanks.
@alberto-sanchez, @mkirsano this is something you requested to watch as well.
@silviodonato, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

Dr15Jones · 2021-06-15T19:02:31Z

please test

Dr15Jones · 2021-06-15T19:05:48Z

GeneratorInterface/Core/python/ExternalGeneratorFilter.py

 class ExternalGeneratorFilter(cms.EDFilter):
-    def __init__(self, prod, _external_process_waitTime_ = cms.untracked.uint32(60), _external_process_verbose_ = cms.untracked.bool(False)):
+    def __init__(self, prod, _external_process_waitTime_ = cms.untracked.uint32(60), _external_process_verbose_ = cms.untracked.bool(False),
+                 _external_process_esModules_ = cms.vstring()):


NOTE: although I called this _external_process_esModules_, technically the module names of any type of plugin could be passed and it would work. It only really makes sense for ES modules (and maybe Services for testing). If you'd rather, this could be changed to _external_process_components_ or something else to capture what is technically possible rather than what I think is most reasonable.

I like the proposal components as it might avoid confusion.

Dr15Jones · 2021-06-15T19:07:33Z

GeneratorInterface/Core/python/ExternalGeneratorFilter.py

        newpset.addString(False,"@python_config", self._prod.dumpPython())
        newpset.addBool(False,"_external_process_verbose_", self._external_process_verbose_.value())
        newpset.addUInt32(False,"_external_process_waitTime_", self._external_process_waitTime_.value())
+        newpset.addVString(True, "_external_process_esModules_", self._external_process_esModules_.value())


The new parameter is tracked since the ES modules loaded affect the physics.

cmsbuild · 2021-06-15T22:07:24Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-159cca/15979/summary.html
COMMIT: ca6cb85
CMSSW: CMSSW_12_0_X_2021-06-15-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/34146/15979/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:

Phase2-hgx287 Next step for V16 geometry of HGCal #34114 @bsunanda: Phase2-hgx287 Next step for V16 geometry of HGCal
[SiPixel Payload Inspector] Quality Maps Improved #34071 @mmusich: [SiPixel Payload Inspector] Quality Maps Improved
Merge .h and .cc files of RecoParticleFlow/PFTracking plugins #34069 @guitargeek: Merge .h and .cc files of RecoParticleFlow/PFTracking plugins
[DB] Apply code-checks/format with misc-definitions-in-headers #34038 @smuzaffar: [DB] Apply code-checks/format with misc-definitions-in-headers

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

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 38
DQMHistoTests: Total histograms compared: 2862520
DQMHistoTests: Total failures: 1
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2862497
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 37 files compared)
Checked 160 log files, 37 edm output root files, 38 DQM output files
TriggerResults: no differences found

colizz · 2021-06-16T02:33:14Z

Many thanks! Following up with @PhyM73 's test in #34117, it now works for P8+Tauola, P8+EvtGen, H7. We plan to also validate under other GeneratorFilter based modules and will reply to this thread soon

cmsbuild · 2021-06-17T13:41:05Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34146/23374

This PR adds an extra 12KB to repository

cmsbuild · 2021-06-17T13:41:25Z

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

colizz · 2021-06-17T14:10:48Z

Hi, from the GEN side, ExternalGeneratorFilter module has been tested on all possible GeneratorFilter-based modules. They all run well and physics results are consistent by now (see link, few are still running, results to be updated). Here's the summary table:

(update: all validations have completed)

Module	Test process	Test command
AMPTGeneratorFilter	workflow 280	`cmsDriver.py AMPT_PPb_5020GeV_MinimumBias_cfi --conditions auto:run1_mc -n 5000 --eventcontent RAWSIM --relval 9000,100 -s GEN --datatier GEN --beamspot Realistic8TeVCollision --fileout file:step1.root --no_exec --nThreads 4`
Herwig7GeneratorFilter (with no LHE step!)	-	`cmsDriver.py GeneratorInterface/Herwig7Interface/DYToLL_TuneCH3_13TeV_herwig7_cff --conditions auto:run2_mc -n 5000 --eventcontent RAWSIM --relval 9000,100 -s GEN --datatier GEN --beamspot Realistic8TeVCollision --fileout file:step1.root --no_exec --nThreads 4`
HydjetGeneratorFilter	workflow 140	`cmsDriver.py Hydjet_Quenched_B12_5020GeV_cfi --conditions auto:run1_mc_hi --scenario HeavyIons -n 5000 --eventcontent RAWSIM --relval 2000,1 -s GEN --datatier GEN --beamspot RealisticHI2011Collision --fileout file:step1.root --no_exec --nThreads 4`
PyquenGeneratorFilter	workflow 301	`cmsDriver.py Pyquen_DiJet_pt80to120_2760GeV_cfi --conditions auto:phase1_2018_realistic_hi --scenario HeavyIons --pileup_input das:/RelValHydjetQ_B12_5020GeV_2018/CMSSW_10_6_0-106X_upgrade2018_realistic_v4-v1/GEN-SIM -n 5000 --era Run2_2018_pp_on_AA --eventcontent RAWSIM --relval 9000,100 -s GEN --pileup HiMixGEN --datatier GEN --beamspot MatchHI --geometry DB:Extended --fileout file:step1.root --no_exec --nThreads 4`
Pythia6GeneratorFilter	workflow 203	`cmsDriver.py H130GGgluonfusion_8TeV_TuneCUETP8M1_cfi --conditions auto:run1_mc -n 5000 --eventcontent RAWSIM --relval 9000,100 -s GEN --datatier GEN --beamspot Realistic8TeVCollision --fileout file:step1.root --no_exec --nThreads 4`
Pythia8GeneratorFilter (pure P8)	workflow 503	`cmsDriver.py MinBias_13TeV_pythia8_cff --conditions auto:run2_mc -s GEN --datatier GEN --beamspot Realistic8TeVCollision -n 5000 --relval 250000,5000 --eventcontent RAWSIM --fileout file:step1.root --no_exec --nThreads 4`
Pythia8GeneratorFilter (P8+EvtGen)	workflow 545	`cmsDriver.py BsToMuMu_forSTEAM_13TeV_cfi --conditions auto:run2_mc -s GEN --datatier GEN --beamspot Realistic8TeVCollision -n 5000 --relval 250000,5000 --eventcontent RAWSIM --fileout file:step1.root --no_exec --nThreads 4`
Pythia8GeneratorFilter (P8+Tauola) (requires `_external_process_components_=cms.vstring("HepPDTESSource")` in ExternalGeneratorFilter)	-	`cmsDriver.py GGToHtautau_13TeV_pythia8_Tauola_taupinu_cff --conditions auto:run2_mc -s GEN --datatier GEN --beamspot Realistic8TeVCollision -n 5000 --relval 250000,5000 --eventcontent RAWSIM --fileout file:step1.root --no_exec --nThreads 4`
Pythia8GeneratorFilter (P8+Photon)	-	Fragment taken as the P8GenFilter block from `GeneratorInterface/PhotosInterface/test/DYphotospp_Tune4C_13TeV_pythia8.py` `cmsDriver.py DYphotospp_Tune4C_13TeV_pythia8_cff --conditions auto:run2_mc -s GEN --datatier GEN --beamspot Realistic8TeVCollision -n 5000 --relval 250000,5000 --eventcontent RAWSIM --fileout file:step1.root --no_exec --nThreads 4`
ReggeGribovPartonMCGeneratorFilter	workflow 531	`cmsDriver.py GeneratorInterface/ReggeGribovPartonMCInterface/ReggeGribovPartonMC_EposLHC_5TeV_pPb_cfi --conditions auto:run2_mc -s GEN --datatier GEN --beamspot Realistic8TeVCollision -n 5000 --relval 250000,5000 --eventcontent RAWSIM --fileout file:step1.root --no_exec --nThreads 4`
SherpaGeneratorFilter	prepid HIG-RunIISummer19UL17GEN-00004	`cmsDriver.py Configuration/GenProduction/python/HIG-RunIISummer19UL17GEN-00004-fragment.py --python_filename HIG-RunIISummer19UL17GEN-00004_1_cfg.py --eventcontent RAWSIM --customise Configuration/DataProcessing/Utils.addMonitoring --datatier GEN --fileout file:step1.root --conditions 112X_mcRun2_asymptotic_v2 --beamspot Realistic25ns13TeVEarly2017Collision --step GEN --geometry DB:Extended --era Run2_2017 --no_exec --mc -n 5000 --nThreads 4`

colizz · 2021-06-17T14:13:28Z

I would suggest a small modification on running the external commands. Some generators will release intermediate files to local dir (e.g. H7, Sherpa) so it would be better to put them in different directories to avoid overlapping. Here is the patch I temporarily added to run the above validation: https://gist.github.com/colizz/76d7e1a4720db5cfc457ee027964c23a

@Dr15Jones If it is fine, would you also help to commit the change to this PR?

Dr15Jones · 2021-06-17T14:22:14Z

@Dr15Jones If it is fine, would you also help to commit the change to this PR?

I’m in vacation now. Please go ahead and make another pull request.

Dr15Jones · 2021-06-17T14:22:27Z

please test

cmsbuild · 2021-06-17T17:27:34Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-159cca/16081/summary.html
COMMIT: 79c69b3
CMSSW: CMSSW_12_0_X_2021-06-16-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/34146/16081/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: 38
DQMHistoTests: Total histograms compared: 2862520
DQMHistoTests: Total failures: 7
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2862491
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 37 files compared)
Checked 160 log files, 37 edm output root files, 38 DQM output files
TriggerResults: no differences found

colizz · 2021-06-17T19:14:55Z

Ok thanks, I make another PR #34173 to fix the working directory issue.

From the GEN side, the comprehensive validation on ExternalGeneratorFilter also completes. We see good agreement w/ or w/o using the ExternalGeneratorFilter as a wrapper. Please see full results in [link].
It can validate that ExtenralGeneratorFilter is generalised to adapt all GeneratorFilter-base modules.

agrohsje · 2021-06-21T13:20:00Z

+generators

cmsbuild · 2021-06-21T13:20:30Z

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

qliphy · 2021-06-21T13:49:21Z

+1

Backport cms-sw#34146 to 10_6_X

Allow specification of ES modules in ExternalGeneratorFilter

ca6cb85

The parameter _external_process_esModules_ allows one to specify the names of ES modules which should be copied from the main process and started in the external process.

cmsbuild added this to the CMSSW_12_0_X milestone Jun 15, 2021

cmsbuild added code-checks-pending generators-pending orp-pending pending-signatures tests-pending labels Jun 15, 2021

Dr15Jones mentioned this pull request Jun 15, 2021

Inaccessiable EventSetup in ExternalGeneratorFilter #34117

Closed

cmsbuild added code-checks-approved and removed code-checks-pending labels Jun 15, 2021

cmsbuild added tests-started and removed tests-pending labels Jun 15, 2021

Dr15Jones commented Jun 15, 2021

View reviewed changes

Dr15Jones mentioned this pull request Jun 15, 2021

Add possibility to access EventSetup from ExternalGeneratorFilter cms-sw/framework-team#168

Closed

cmsbuild added tests-approved and removed tests-started labels Jun 15, 2021

changed new argument from *_esModules_ to *_components_

79c69b3

cmsbuild added code-checks-pending tests-pending and removed code-checks-approved tests-approved labels Jun 17, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Jun 17, 2021

cmsbuild added tests-started and removed tests-pending labels Jun 17, 2021

cmsbuild added tests-approved and removed tests-started labels Jun 17, 2021

colizz mentioned this pull request Jun 17, 2021

Specify separate directories to run external instances in ExternalGeneratorFilter #34173

Merged

cmsbuild added fully-signed generators-approved and removed generators-pending pending-signatures labels Jun 21, 2021

cmsbuild added orp-approved and removed orp-pending labels Jun 21, 2021

cmsbuild merged commit 3d3c054 into cms-sw:master Jun 21, 2021

Dr15Jones deleted the extraConfigForExternalGeneratorFilter branch June 28, 2021 15:19

colizz added a commit to colizz/cmssw that referenced this pull request Jul 16, 2021

Allow specification of ES modules in ExternalGeneratorFilter (10_6_X)

665b0b4

Backport cms-sw#34146 to 10_6_X

colizz mentioned this pull request Jul 16, 2021

[10_6_X] Backport concurrent GEN utilities #34527

Merged

Allow specification of ES modules in ExternalGeneratorFilter #34146

Allow specification of ES modules in ExternalGeneratorFilter #34146

Uh oh!

Conversation

Dr15Jones commented Jun 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR description:

PR validation:

Uh oh!

cmsbuild commented Jun 15, 2021

Uh oh!

cmsbuild commented Jun 15, 2021

Uh oh!

Dr15Jones commented Jun 15, 2021

Uh oh!

Dr15Jones Jun 15, 2021

Choose a reason for hiding this comment

Uh oh!

agrohsje Jun 17, 2021

Choose a reason for hiding this comment

Uh oh!

Dr15Jones Jun 15, 2021

Choose a reason for hiding this comment

Uh oh!

cmsbuild commented Jun 15, 2021

Comparison Summary

Uh oh!

colizz commented Jun 16, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cmsbuild commented Jun 17, 2021

Uh oh!

cmsbuild commented Jun 17, 2021

Uh oh!

colizz commented Jun 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

colizz commented Jun 17, 2021

Uh oh!

Dr15Jones commented Jun 17, 2021

Uh oh!

Dr15Jones commented Jun 17, 2021

Uh oh!

cmsbuild commented Jun 17, 2021

Comparison Summary

Uh oh!

colizz commented Jun 17, 2021

Uh oh!

agrohsje commented Jun 21, 2021

Uh oh!

cmsbuild commented Jun 21, 2021

Uh oh!

qliphy commented Jun 21, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Dr15Jones commented Jun 15, 2021 •

edited

Loading

colizz commented Jun 16, 2021 •

edited

Loading

colizz commented Jun 17, 2021 •

edited

Loading