search

cms-sw / cmssw

CMS Offline Software
http://cms-sw.github.io/
Apache License 2.0
1.08k stars 4.31k forks source link

Access edm::event from GeneratorInterface #28636

Closed wouf closed 4 years ago

wouf commented 4 years ago

Three HI generators has an issue with edm::event access in embedding mode. It trying to read empty edm::event using BaseHadroniser::getEDMEvent function: Pyquen: https://github.com/cms-sw/cmssw/blob/master/GeneratorInterface/PyquenInterface/src/PyquenHadronizer.cc#L125 Hydjet2: https://github.com/cms-sw/cmssw/blob/master/GeneratorInterface/Hydjet2Interface/src/Hydjet2Hadronizer.cc#L491 Hydjet: https://github.com/cms-sw/cmssw/blob/master/GeneratorInterface/HydjetInterface/src/HydjetHadronizer.cc#L195

To correct it it is needed call BaseHadroniser::setEDMEvent somewhere, but for now I can't to figure out how to access edm::event (from HiMixGEN) in GeneratorFilter before event generation.

cmsbuild commented 4 years ago

A new Issue was created by @wouf .

@davidlange6, @Dr15Jones, @smuzaffar, @fabiocos, @kpedro88 can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

Dr15Jones commented 4 years ago

assign generator

Dr15Jones commented 4 years ago

assign generators

cmsbuild commented 4 years ago

New categories assigned: generators

@alberto-sanchez,@SiewYan,@qliphy,@efeyazgan,@mkirsano,@agrohsje you have been requested to review this Pull request/Issue and eventually sign? Thanks

wouf commented 4 years ago

Any ideas on this?

qliphy commented 4 years ago

@wouf Would you please give more details about this issue? What is embedding mode and why this was not used before? and do you have a recipe to reproduce the error?

wouf commented 4 years ago

@qliphy The embedding mode on generator level, is needed to mix heavy ion events: to do it we have to care about impact parameter and event plane angle - it have to be the same for both events. To mix two MinBias samples we have to generate the second one using the first as underlying. For HYDJET, on each event (before generating) we have to read underlying sample event like https://github.com/cms-sw/cmssw/blob/master/GeneratorInterface/HydjetInterface/src/HydjetHadronizer.cc#L195 to get impact parameter and event plane angle L201, L202 used as input in HYDJET. Before it was not used with HYDJET, because the only last version (from 28 November 2019) support this feature.

To reproduce it with HYDJET one have to use something like cmsDriver.py Hydjet_Quenched_MinBias_5020GeV_cfi --conditions auto:phase1_2018_realistic --scenario HeavyIons -n 1 --era Run2_2018 --eventcontent RAWSIM --relval 2000,1 -s GEN,SIM --datatier GEN-SIM --beamspot Realistic25ns13TeVEarly2018Collision --geometry DB:Extended --pileup HiMixGEN --pileup_input file:Hydjet_Quenched_B12_5020GeV_cfi_GEN_SIM.root --fileout test_GEN_SIM_PU.root --no_exec And set embeddingMode = cms.bool(True). Then just print the size of event е just after L195

Or one can just look through BaseHadroniser code and realize that we have just empty edm::event, before we care about setEDMEvent.

qliphy commented 4 years ago

Thanks! Can you point us to the input file?

wouf commented 4 years ago

@qliphy It may be any generated in HepMC format with hi section. I created it with: cmsDriver.py Hydjet_Quenched_B12_5020GeV_cfi --conditions auto:phase1_2018_realistic --scenario HeavyIons -n 1 --era Run2_2018 --eventcontent RAWSIM --relval 2000,1 -s GEN,SIM --datatier GEN-SIM --beamspot Realistic25ns13TeVEarly2018Collision --geometry DB:Extended

qliphy commented 4 years ago

Thanks @wouf I tried to reproduce your error:

After producing an input file, and then proceed to mix the events, I got (1) cmsRun Hydjet_Quenched_MinBias_5020GeV_cfi_GEN_SIM_PU.py ----- Begin Fatal Exception 20-Jan-2020 09:08:41 CET----------------------- An exception of category 'Configuration' occurred while [0] Constructing the EventProcessor [1] Constructing module: class=HydjetGeneratorFilter label='generator' Exception Message: MissingParameter: Parameter 'backgroundLabel' not found.

Then I added in the python file as below: backgroundLabel = cms.InputTag("generator","unsmeared")

(2) then I got .... Impact parameter larger than three nuclear radius! Begin processing the 1st record. Run 1, Event 1, LumiSection 1 on stream 0 at 20-Jan-2020 09:13:34.079 CET ----- Begin Fatal Exception 20-Jan-2020 09:13:34 CET----------------------- An exception of category 'GetByLabelWithoutRegistration' occurred while [0] Processing Event run: 1 lumi: 1 event: 1 stream: 0 [1] Running path 'simulation_step' [2] Calling method for module HydjetGeneratorFilter/'generator' Exception Message: ::getByLabel without corresponding call to consumes or mayConsumes for this module. type: edm::HepMCProduct module label: generator product instance name: 'unsmeared' process name: ''

Have you already added consumes syntax to replace getByLabel by getByToken? https://github.com/cms-sw/cmssw/blob/master/GeneratorInterface/HydjetInterface/src/HydjetHadronizer.cc#L197

wouf commented 4 years ago

I added one to #28764

qliphy commented 4 years ago

Thanks @wouf
However, when I tried as below, it doesn't compile:

cmsrel CMSSW_11_1_0_pre1 git cms-addpkg GeneratorInterface/HydjetInterface git cms-merge-topic wouf:from-CMSSW_11_0_0_pre7

/tmp/qili/XXX/CMSSW_11_1_0_pre1/src/GeneratorInterface/HydjetInterface/src/HydjetHadronizer.cc: In constructor 'gen::HydjetHadronizer::HydjetHadronizer(const edm::ParameterSet&)': /tmp/qili/XXX/CMSSW_11_1_0pre1/src/GeneratorInterface/HydjetInterface/src/HydjetHadronizer.cc:106:12: error: 'mayConsume' was not declared in this scope src = mayConsume(iConfig.getUntrackedParameter( "backgroundLabel", edm::InputTag("generator","unsmeared") )); ^~~~~~ /tmp/qili/XXX/CMSSW_11_1_0pre1/src/GeneratorInterface/HydjetInterface/src/HydjetHadronizer.cc:106:35: error: expected primary-expression before '>' token src = mayConsume(iConfig.getUntrackedParameter( "backgroundLabel", edm::InputTag("generator","unsmeared") )); ^ /tmp/qili/XXX/CMSSW_11_1_0pre1/src/GeneratorInterface/HydjetInterface/src/HydjetHadronizer.cc:106:37: error: 'iConfig' was not declared in this scope src = mayConsume(iConfig.getUntrackedParameter( "backgroundLabel", edm::InputTag("generator","unsmeared") ));

wouf commented 4 years ago

@qliphy I'm sorry, I committed wrong project. Now fixed.

qliphy commented 4 years ago

Thanks, now I get other error messages as below:

/cvmfs/cms.cern.ch/slc7_amd64_gcc820/external/gcc/8.2.0-pafccj/bin/../lib/gcc/x86_64-unknown-linux-gnu/8.3.1/../../../../x86_64-unknown-linux-gnu/bin/ld: tmp/slc7_amd64_gcc820/src/GeneratorInterface/HydjetInterface/src/GeneratorInterfaceHydjetInterface/HydjetHadronizer.cc.o: in function gen::HydjetHadronizer::add_heavy_ion_rec(HepMC::GenEvent*)': HydjetHadronizer.cc:(.text+0x95): undefined reference tohyfpar_' /cvmfs/cms.cern.ch/slc7_amd64_gcc820/external/gcc/8.2.0-pafccj/bin/../lib/gcc/x86_64-unknown-linux-gnu/8.3.1/../../../../x8664-unknown-linux-gnu/bin/ld: HydjetHadronizer.cc:(.text+0x15f): undefined reference to `hyjpar' /cvmfs/cms.cern.ch/slc7_amd64_gcc820/external/gcc/8.2.0-pafccj/bin/../lib/gcc/x86_64-unknown-linux-gnu/8.3.1/../../../../x86_64-unknown-linux-gnu/bin/ld: tmp/slc7_amd64_gcc820/src/GeneratorInterface/HydjetInterface/src/GeneratorInterfaceHydjetInterface/HydjetHadronizer.cc.o: in function gen::HydjetHadronizer::call_hyinit(double, double, int, double, double, double, int)': HydjetHadronizer.cc:(.text+0x231): undefined reference tohyinit_' /cvmfs/cms.cern.ch/slc7_amd64_gcc820/external/gcc/8.2.0-pafccj/bin/../lib/gcc/x86_64-unknown-linux-gnu/8.3.1/../../../../x86_64-unknown-linux-gnu/bin/ld: tmp/slc7_amd64_gcc820/src/GeneratorInterface/HydjetInterface/src/GeneratorInterfaceHydjetInterface/HydjetHadronizer.cc.o: in function gen::HydjetHadronizer::build_hyjet_vertex(int, int)': HydjetHadronizer.cc:(.text+0x373): undefined reference tohyjets_'

wouf commented 4 years ago

@qliphy Are You tested it with external hydjet?

wouf commented 4 years ago

The version below 1.9.1 does not support "embedding mode"

wouf commented 4 years ago

@qliphy were you able to reproduce the issue?

qliphy commented 4 years ago

Not yet, Still trying.

wouf commented 4 years ago

@qliphy I can provide my sequence for local testing if You need

qliphy commented 4 years ago

@wouf that would be useful indeed.

wouf commented 4 years ago

@qliphy sorry for the late replay

On lxplus7 (in the test folder) please do: export SCRAM_ARCH=slc7_amd64_gcc700 source /cvmfs/sft.cern.ch/lcg/contrib/gcc/7.2.0/x86_64-centos7-gcc7-opt/setup.sh

Then run the script: git clone https://github.com/wouf/scripts.git sh scripts/TestHydjet.sh

Then get hydjet 1.9.1 interface: scram project CMSSW CMSSW_11_0_0_pre7 cd CMSSW_11_0_0_pre7/src cmsenv git cms-addpkg GeneratorInterface/HydjetInterface git cms-merge-topic wouf:from-CMSSW_11_0_0_pre7

Then setup hydjet libs: scram setup ../../testFolder/slc7_amd64_gcc700/external/hydjet-toolfile/1.0/etc/scram.d/hydjet.xml scram b

To check if it is really version 1.9.1, please run the test: cmsRun GeneratorInterface/HydjetInterface/test/testHydjet.py

qliphy commented 4 years ago

@wouf I confirm I can reproduce the error.

(1) In Hydjet_Quenched_MinBias_5020GeV_cfi_GEN_SIM_PU.py, there is

filter all path with the production filter sequence

      for path in process.paths:
             **getattr(process,path).insert(0, process.generator)**

where process.generator runs first: process.generator = cms.EDFilter("HydjetGeneratorFilter", ....

(2) The relevant pgen path is defined as here: process.generation_step = cms.Path(process.pgen) pgen = cms.Sequence(cms.SequencePlaceholder("randomEngineStateProducer")+cms.SequencePlaceholder("mix")+VertexSmearing+GenSmeared+genParticles+hiGenJets)

(3) and thus the mixing module runs after generator path process.mix.input.fileNames = cms.untracked.vstring(['file:Hydjet_Quenched_B12_5020GeV_cfi_GEN_SIM.root'])

(4) All these should explain why currently getEDMEvent gets empty, and probably setEDMEvent should be called after/in the mixing module, e.g.: https://github.com/cms-sw/cmssw/blob/master/SimGeneral/MixingModule/plugins/MixingModule.cc#L169-L175 and one further adjusts a bit above sequence order.

Hope software experts @davidlange6, @Dr15Jones, @smuzaffar, @fabiocos, @kpedro88 @mkirsano @alberto-sanchez @SiewYan can have a look and give more suggestion or help.

Dr15Jones commented 4 years ago

@makortel FYI

makortel commented 4 years ago

I'm pretty sure I'm missing something so please correct me

From the discussion above it is not clear to me what exactly is the problem. Is it that getEDMEvent() returns "garbage" (that would imply setEDMEvent() call being missing)? Or that the edm::Event itself gets propagated fine, but is missing some product (that would imply ordering problem in the Sequence)? Or something else?

wouf commented 4 years ago

@makortel I think getEDMEvent() and setEDMEvent() works as expected (for filter after each event). So by constructed getEDMEvent() should return current generated event. But: we think if it is possible to use it before generating (for each event) to read underlying one. The purpose is to read each event from input sample from mixing module, before generating each event. I think using setEDMEvent()/getEDMEvent() before each next event and after previous event filters is the bеtter then to introduce the new edm::Event instance to the BaseHadroniser.

makortel commented 4 years ago

I'm still confused. Do I understand correctly that

?

If this is the case, then the sequence should have process.mix before process.generator (*). The HydjetGeneratorFilter should then read CrossingFrame<edm::HepMCProduct> (produced by MixingModule) from edm::Event. The CrossingFrame contains the the edm::HepMCProduct for each mixed "original event", and thus HydjetGeneratorFilter can do whatever it needs to do with them.

(*) even better would be that all producers in the generator sequence would be moved into a cms.Task and only the filters would be left into the cms.Sequence. Then framework would automatically run the producers in the order of data dependencies.

wouf commented 4 years ago

@makortel the two streams (event by event) needed by mixing module:

We can't set process.mix before process.generator: we have to generate something before mix it. We just want to get access to the sample from process.mix.input.fileNames during generating.

Please let me know if anything else is not clear.

makortel commented 4 years ago

@wouf Thanks for the clarifications. If process.generator needs the "underlying input event", is the process.mix itself needed? I.e. is something else then using the CrossingFrame<edm::HepMCProduct>?

wouf commented 4 years ago

@makortel mixing module needed to mix events. Generator does not mix its itself. By using underlying sample (from process.mix.input.fileNames) it just synchronize two HI specific input parameters (impact parameter and event plain angle) for each mixed pair.

makortel commented 4 years ago

The thing is that the MixingModule can not give access to the events from process.mix.input.fileNames outside MixingModule. So one option that came to my mind would be to run the generator within the MixingModule.

Another option would be to MixingModule to mix empty signal with the underlying sample, then HydjetGeneratorFilter to read that and produce another CrossingFrame<edm::HepMCProduct>.

Is there always exactly one event read from the underlying sample for each signal event? If yes, one option would be to read the underlying sample through PoolSource, and generate CrossingFrame<edm::HepMCProduct>.

Do the "impact parameter and event plain angle" change event by event?

wouf commented 4 years ago

@makortel Yes one underlying event for each generated (with the same event number). The impact parameter and event plane angle, changed respectively corresponding (with the same ordinal number) underlying event. Do we able to use PoolSource? In this case we have to change mixing module? I think Your second case is too "heavy". Which one do You think would be the better case (the generating speed is in priority)?

makortel commented 4 years ago

To me the PoolSource approach sounds the simplest solution (unless I miss some conflicting detail in the procedure). The job reads the underlying event data as a regular input, the generator reads what it needs from the input, and produces what it produces after that. MixingModule would not be needed. Assuming it is still simplest to keep the downstream to consume CrossingFrame<edm::HepMCProduct> two options come to my mind

wouf commented 4 years ago

@makortel we need MixingModule to mixing. We want to avoid saving generated sample and then mixing it with underlying one. We want to generate and mix it "on fly". In case we will use PoolSource, the access to the underlying .root file would be blocked for MixingModule. Мoreover all workflows with --pileup HiMixGEN --pileup_input assuming that --pileup_input will be transmitted to process.mix.input.fileNames. It's not a trivial task given the fact that we have to save (or change it for all) workflow constructions as: Hydjet_Quenched_MinBias_5020GeV_cfi --conditions auto:phase1_2018_realistic --scenario HeavyIons -n 1 --era Run2_2018 --eventcontent RAWSIM --relval 2000,1 -s GEN,SIM --datatier GEN-SIM --beamspot Realistic25ns13TeVEarly2018Collision --geometry DB:Extended --pileup HiMixGEN --pileup_input file:Hydjet_Quenched_B12_5020GeV_cfi_GEN_SIM.root --fileout step333_GEN_SIM_PU.root --no_exec

makortel commented 4 years ago

If the MixingModule is really needed (I must admit I haven't fully understood the logic of the mixing of edm::HepMCProduct) the only options I can think of are

So basically the problem is that there would be a cyclic dependence between HydjetGeneratorFilter and MixingModule, and that cycle needs to be broken somehow.

wouf commented 4 years ago

@makortel It looks like Your way would break workflow order and may be to slow for generating.

  1. The first my idea was to use setEDMEvent from the place where the stream opened. When we run the workflow above, it's clearly seen that stream opened just after generator constructor (before generating!):
cmsRun Hydjet_MY_Quenched_MinBias_5020GeV_cfi_GEN_SIM_PU.py 
 %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
 %%                      This is HYDJET version 1.9.1                        %%
 %%                      with using PYQUEN version 1.5.3                     %%
 %%         Corresponding author: Igor Lokhtin (Igor.Lokhtin@cern.ch)        %%
 %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
**29-Jan-2020 09:02:04 CET  Initiating request to open file file:../../CMSSW_10_3_2/src/Hydjet_Quenched_B12_5020GeV_cfi_GEN_SIM.root
29-Jan-2020 09:02:06 CET  Successfully opened file file:../../CMSSW_10_3_2/src/Hydjet_Quenched_B12_5020GeV_cfi_GEN_SIM.root**
1                                                                              
 ******************************************************************************
 ******************************************************************************
 **                                                                          **
 **                                                                          **
 **              *......*                  Welcome to the Lund Monte Carlo!  **
 **         *:::!!:::::::::::*                                               **
 **      *::::::!!::::::::::::::*          PPP  Y   Y TTTTT H   H III   A    **
 **    *::::::::!!::::::::::::::::*        P  P  Y Y    T   H   H  I   A A   **
 **   *:::::::::!!:::::::::::::::::*       PPP    Y     T   HHHHH  I  AAAAA  **
 **   *:::::::::!!:::::::::::::::::*       P      Y     T   H   H  I  A   A  **
 **    *::::::::!!::::::::::::::::*!       P      Y     T   H   H III A   A  **
 **      *::::::!!::::::::::::::* !!                                         **
  1. Other idea (in case the first one is impossible) is to create the new module which would open the --pileup stream before generating and mixing. The generator will access it and read needed information, then stream would be directed as input for mixing module. To the mixing module will need to add option to read stream instead of create it itself. In this case the order would be the standard: the only thing input should be smarter.

2.1 Or to change it for all chains. In this case we just move the stream creation from MixingModule to standalone one, which would be started before generation.

Do You think it's possible?

makortel commented 4 years ago
  1. The first my idea was to use setEDMEvent from the place where the stream opened. When we run the workflow above, it's clearly seen that stream opened just after generator constructor (before generating!):

The problem with this approach is that it is not possible to call setEDMEvent "from the place the file is opened". The file is opened by MixingModule, and the setEDMEvent is called within HydjetGeneratorFilter. These two modules should not communicate except via edm::Event (or edm::LuminosityBlock or edm::Run), and should have a strict consumer-producer relationship (within one transition type).

  1. Other idea (in case the first one is impossible) is to create the new module which would open the --pileup stream before generating and mixing. The generator will access it and read needed information, then stream would be directed as input for mixing module. To the mixing module will need to add option to read stream instead of create it itself. In this case the order would be the standard: the only thing input should be smarter.

We don't have any mechanism for two modules to share a set of files (technically EmbeddedRootSource). In addition this approach would require teaching cmsDriver/ConfigBuilder that in some cases the --pileup should be set to other module than process.mix, which would make an already very complex code even more complex.

2.1 Or to change it for all chains. In this case we just move the stream creation from MixingModule to standalone one, which would be started before generation.

I don't quite follow what exactly do you refer to with "chains" or "standalone one". Could you elaborate?

If I understood your 2.1 correctly, I think my "PoolSource" option from https://github.com/cms-sw/cmssw/issues/28636#issuecomment-579417714 and https://github.com/cms-sw/cmssw/issues/28636#issuecomment-579450023 is not that far away. Let me describe it in more detail

  1. The "underlying events" or "stream" is read by PoolSource as regular input. The data products from these events are available for every module via edm::Event. The HydjetGeneratorFilter reads the "underlying edm::HepMCProduct" from edm::Event, and produces a new edm::HepMCProduct into the edm::Event. Then, a new module reads both the "underlying edm::HepMCProduct" and the just-generated edm::HepMCProduct, and produces CrossingFrame<edm::HepMCProduct> just like MixingModule would do (the MixingModule logic for edm::HepMCProduct is not really technically complicated). You're right that this approach would imply a change in the cmsDriver arguments from --pileup HiMixGEN --pileup_input file:Hydjet_Quenched_B12_5020GeV_cfi_GEN_SIM.root to --filein file:Hydjet_Quenched_B12_5020GeV_cfi_GEN_SIM.root, but to me that looks straightforward.
wouf commented 4 years ago

@makortel In my 2.1 I speak about to move calling of EmbeddedRootSource outside of MixingModule and to teaching cmsDriver/ConfigBuilder to call it before generator. Then just to share each event via edm::Event for generator and mixing module both. But I'm not sure if it is possible for SecSource. In that case we would need to make changes in MixingModule as well.

As far as I understand Your point is creation of plugin in GeneratorInterface/**AnyGenerator**Interface/plugins the same way as /SimGeneral/MixingModule/plugins/MixingModule.cc, to mixing and producing CrossingFrame<edm::HepMCProduct> in GeneratorFilfer itself?

makortel commented 4 years ago

In my 2.1 I speak about to move calling of EmbeddedRootSource outside of MixingModule and to teaching cmsDriver/ConfigBuilder to call it before generator.

EmbeddedRootSource will not be moved outside of MixingModule because every other use of MixingModule needs it.

Then just to share each event via edm::Event for generator and mixing module both. But I'm not sure if it is possible for SecSource.

Indeed that is not possible. The SecSource is an implementation detail of MixingModule that does not get exposed outside of it. Also the "full event" from SecSource can not be stored in the "main" edm::Event, but (pointers to) individual products of the SecSource events can be (e.g. CrossingFrame<T> does that).

In that case we would need to make changes in MixingModule as well.

I think it would also be much easier to create a new module "mixing" the edm::HepMCProduct in absence of EmbeddedRootSource than to modify MixingModule to not use the EmbeddedRootSource.

As far as I understand Your point is creation of plugin in GeneratorInterface/**AnyGenerator**Interface/plugins the same way as /SimGeneral/MixingModule/plugins/MixingModule.cc, to mixing and producing CrossingFrame<edm::HepMCProduct> in GeneratorFilfer itself?

That is one option. An alternative would be to leave the generators to produce edm::HepMCProduct and mix and produce the CrossingFrame<edm::HepMCProduct> in a new, dedicated module (that probably should reside in SimGeneral/MixingModule package).

wouf commented 4 years ago

@makortel Thanks for explanation!

So, if I understand Your correctly, we have 2 ways:

  1. To create new module "mixing" in absence of EmbeddedRootSource. The sequence would be EmbeddedRootSource -> generator -> NewMixingModule (cmsDriver/ConfigBuilder must be trained). The question is how the "underlying events" can go through this chain. The cmsDriver command would be something like --pileup HiMixEmbGEN --pileup_input file:.... The NewMixingModule will be run by new config HiMixEmbGEN_cff.py (residing in SimGeneral/MixingModule package)

  2. Creating the other type of NewMixingModule which would using PoolSource as "underlying input". In this case the sequence would be PoolSource -> generator -> NewMixingModule (the NewMixingModule may be called from the generator's filter). The cmsDriver command should contain ... --filein file:... and does not --pileup. The mixing on/off switch should be the generator's parameter.

I'd like the first one: more clear for users.

makortel commented 4 years ago
  1. To create new module "mixing" in absence of EmbeddedRootSource. The sequence would be EmbeddedRootSource -> generator -> NewMixingModule (cmsDriver/ConfigBuilder must be trained).

That would really be MixingModule -> generator -> NewMixingModule where MixingModule contains the EmbeddedRootSource.

The question is how the "underlying events" can go through this chain.

They would go in the pileup part of CrossingFrame<edm::HepMCProduct>.

The cmsDriver command would be something like --pileup HiMixEmbGEN --pileup_input file:.... The NewMixingModule will be run by new config HiMixEmbGEN_cff.py (residing in SimGeneral/MixingModule package)

The HiMixEmbGEN_cff.py would have to contain the definition for MixingModule in this "empty signal event mixing" case.

  1. Creating the other type of NewMixingModule which would using PoolSource as "underlying input". In this case the sequence would be PoolSource -> generator -> NewMixingModule (the NewMixingModule may be called from the generator's filter).

If NewMixingModule is a framework module, it would be called by the framework. In this case the generator module output can stay as edm::HepMCProduct.

If NewMixingModule is made part of the generator module, then the generator module would have to produce CrossingFrame<edm::HepMCProduct>.

The cmsDriver command should contain ... --filein file:... and does not --pileup. The mixing on/off switch should be the generator's parameter.

Is there any other use case for --pileup HiMixGEN than this embedding workflow?

makortel commented 4 years ago

Out of curiosity, why are the underlying MinBias events read from existing files instead of generating them on the fly?

wouf commented 4 years ago

Out of curiosity, why are the underlying MinBias events read from existing files instead of generating them on the fly?

Usually we have central production MinBias sample for multipurpose. Of course the aim is to use DAS input instead of single file.

makortel commented 4 years ago

Out of curiosity, why are the underlying MinBias events read from existing files instead of generating them on the fly?

Usually we have central production MinBias sample for multipurpose. Of course the aim is to use DAS input instead of single file.

Ok, but how long does it take to generate a MinBias event (e.g. compared to the signal event generation)? The I/O has a non-negligible cost as well (and for pp if we would mix signal with a single MinBias event at the generator level, we would almost certainly generate the MinBias on the fly)

wouf commented 4 years ago

Out of curiosity, why are the underlying MinBias events read from existing files instead of generating them on the fly?

Usually we have central production MinBias sample for multipurpose. Of course the aim is to use DAS input instead of single file.

Ok, but how long does it take to generate a MinBias event (e.g. compared to the signal event generation)? The I/O has a non-negligible cost as well (and for pp if we would mix signal with a single MinBias event at the generator level, we would almost certainly generate the MinBias on the fly)

@gsfs George could You please to comment? Thanks!

wouf commented 4 years ago
  1. To create new module "mixing" in absence of EmbeddedRootSource. The sequence would be EmbeddedRootSource -> generator -> NewMixingModule (cmsDriver/ConfigBuilder must be trained).

That would really be MixingModule -> generator -> NewMixingModule where MixingModule contains the EmbeddedRootSource.

The question is how the "underlying events" can go through this chain.

They would go in the pileup part of CrossingFrame<edm::HepMCProduct>.

The cmsDriver command would be something like --pileup HiMixEmbGEN --pileup_input file:.... The NewMixingModule will be run by new config HiMixEmbGEN_cff.py (residing in SimGeneral/MixingModule package)

The HiMixEmbGEN_cff.py would have to contain the definition for MixingModule in this "empty signal event mixing" case.

  1. Creating the other type of NewMixingModule which would using PoolSource as "underlying input". In this case the sequence would be PoolSource -> generator -> NewMixingModule (the NewMixingModule may be called from the generator's filter).

If NewMixingModule is a framework module, it would be called by the framework. In this case the generator module output can stay as edm::HepMCProduct.

If NewMixingModule is made part of the generator module, then the generator module would have to produce CrossingFrame<edm::HepMCProduct>.

The cmsDriver command should contain ... --filein file:... and does not --pileup. The mixing on/off switch should be the generator's parameter.

Is there any other use case for --pileup HiMixGEN than this embedding workflow?

I don't know any other use. Why You are asking?

makortel commented 4 years ago

Is there any other use case for --pileup HiMixGEN than this embedding workflow?

I don't know any other use. Why You are asking?

To understand the level of constraints / freedom on changing things. If this is indeed the only use case, it is easier to change things because there is no need to preserve the old behavior.

gkrintir commented 4 years ago

Out of curiosity, why are the underlying MinBias events read from existing files instead of generating them on the fly?

Usually we have central production MinBias sample for multipurpose. Of course the aim is to use DAS input instead of single file.

Ok, but how long does it take to generate a MinBias event (e.g. compared to the signal event generation)? The I/O has a non-negligible cost as well (and for pp if we would mix signal with a single MinBias event at the generator level, we would almost certainly generate the MinBias on the fly)

@gsfs George could You please to comment? Thanks!

hi @wouf let me chime in for the generation time;

  1. HIN MC contacts are C. Lindsey & L.Palomo, but @gsfs is welcome to reply of course ;D
  2. It should depend on the campaign too; it's ok estimating it for the PbPb 2018 campaign?
  3. Comparing to 'signal' it is also kind of nontrivial; we might have some simply pythia events vs complex events, e.g., ttbar with MG5. Do you have a preference for comparing MinBias against some specif class of events?
wouf commented 4 years ago

Is there any other use case for --pileup HiMixGEN than this embedding workflow?

I don't know any other use. Why You are asking?

To understand the level of constraints / freedom on changing things. If this is indeed the only use case, it is easier to change things because there is no need to preserve the old behavior.

@mandrenguyen Could You please to comment or address to one may know? Thank You!

wouf commented 4 years ago
  1. To create new module "mixing" in absence of EmbeddedRootSource. The sequence would be EmbeddedRootSource -> generator -> NewMixingModule (cmsDriver/ConfigBuilder must be trained).

That would really be MixingModule -> generator -> NewMixingModule where MixingModule contains the EmbeddedRootSource.

The question is how the "underlying events" can go through this chain.

They would go in the pileup part of CrossingFrame<edm::HepMCProduct>.

With the edm::Enent? But the main question is how access edm::Event from GeneratorInterface? The same way as consumesCollector?

wouf commented 4 years ago

Out of curiosity, why are the underlying MinBias events read from existing files instead of generating them on the fly?

Usually we have central production MinBias sample for multipurpose. Of course the aim is to use DAS input instead of single file.

Ok, but how long does it take to generate a MinBias event (e.g. compared to the signal event generation)? The I/O has a non-negligible cost as well (and for pp if we would mix signal with a single MinBias event at the generator level, we would almost certainly generate the MinBias on the fly)

@gsfs George could You please to comment? Thanks!

hi @wouf let me chime in for the generation time;

1. HIN MC contacts are C. Lindsey &  L.Palomo, but @gsfs is welcome to reply of course ;D

2. It should depend on the campaign too; it's ok estimating it for the PbPb 2018 campaign?

3. Comparing to 'signal' it is also kind of nontrivial; we might have some simply pythia events vs complex events, e.g., ttbar with MG5. Do you have a preference for comparing MinBias against some specif class of events?

@gkrintir Dear Georgios 1) I know, I asked George because of his big experience. Hе may know if this was discussed before. 2, 3) The question was about the reasonable of current approach. So we need just an average score.