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

bug fix to read non-edm hepmc files in 71 #18917

Merged
merged 2 commits into from Jul 19, 2017

Conversation

perrozzi
Copy link
Contributor

@perrozzi perrozzi commented May 24, 2017

basically substitute string with inputTag
code adapted from @amarini

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @perrozzi for CMSSW_7_1_X.

It involves the following packages:

IOMC/Input
SimG4Core/Application

@smuzaffar, @civanch, @Dr15Jones, @mdhildreth, @cmsbuild, @davidlange6 can you please review it and eventually sign? Thanks.
@makortel this is something you requested to watch as well.
@davidlange6 you are the release manager for this.

cms-bot commands are listed here

@perrozzi
Copy link
Contributor Author

@amarini @intrepid42 trying to move this to cmssw, feel free to comment

@perrozzi
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented May 24, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/20071/console Started: 2017/05/24 18:42

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@@ -32,7 +33,8 @@ MCFileSource::MCFileSource(const ParameterSet & pset, InputSourceDescription con
}

reader_->initialize(fileName);
produces<HepMCProduct>();
produces<HepMCProduct>("generator");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why change the product instance label?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if I remember correctly, if otherwise the following stage (SIM) gives an error because expects "generator"
@amarini can you please comment?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we still need this later on:

process.g4SimHits.Generator.HepMCProductLabel = 'source:generator'
process.genParticles.src = 'source:generator'

(instead of just source, so it does not make too much difference)

Or is it possible to label the source module as process.generator?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sources are not allowed to have a label. However, Source are allowed to provide data products from 'earlier' Process. Therefore it would be possible to have the source create a data product using the 'generator' module label for a made up earlier process (say 'MCFileGen'). However the base class used by MCFileSource does not support creation of 'earlier' Processes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a solution then?!?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I modified the following stages as well in the code to take generic Input tags that can be configured in python when running

process.genParticles.src= cms.InputTag("source","generator")
process.g4SimHits.HepMCProductLabel = cms.InputTag("source","generator")
process.g4SimHits.Generator.HepMCProductLabel = cms.InputTag("source","generator")

and as well the output commands needs to retain the gen information:

process.AODSIMoutput.outputCommands.extend([
		'keep GenRunInfoProduct_*_*_*',
        	'keep GenLumiInfoProduct_*_*_*',
		'keep GenEventInfoProduct_*_*_*',
		])

process.MINIAODSIMoutput.outputcommands.extend([
       'keep GenRunInfoProduct_*_*_*',
       'keep GenLumiInfoProduct_*_*_*',
       'keep GenEventInfoProduct_*_*_*',
])

The choice to mark it with label 'generator' was for clarity since 'source' is not a very descriptive collection name.
Anyhow, if put or not, the changes to InputTags are still needed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The LHESource does handle creating an 'earlier' Process and then adding a different module label for the data products it makes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so just to be sure I follow - the vertex smearing is applied before this module runs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if I understand correctly your question, the module concerned by this PR needs to run before the vertex smearing as is needed to import into EDM format a showered set of events in HepMC format

@cmsbuild
Copy link
Contributor

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-18917/20071/summary.html

Comparison Summary:

  • You potentially added 8 lines to the logs
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 13
  • DQMHistoTests: Total histograms compared: 842166
  • DQMHistoTests: Total failures: 3940
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 837758
  • DQMHistoTests: Total skipped: 468
  • DQMHistoTests: Total Missing objects: 0
  • Checked 51 log files, 14 edm output root files, 13 DQM output files

@mseidel42
Copy link
Contributor

I just noticed that genParticles are not written into the edm file :s

@amarini
Copy link
Contributor

amarini commented May 26, 2017

Standard sequences can produce gen particles from edm HepMC files.

I was using something like:

process.GEN = cms.OutputModule("PoolOutputModule",
		        fileName = cms.untracked.string('HepMC_GEN.root')
			)
process.load('Configuration.StandardSequences.Services_cff')
process.load('SimGeneral.HepPDTESSource.pythiapdt_cfi')
process.load('GeneratorInterface.Core.genFilterSummary_cff')
process.load('Configuration.StandardSequences.Generator_cff')
process.genParticles.src= cms.InputTag("source","generator")

process.p = cms.Path(process.genParticles)
process.outpath = cms.EndPath(process.GEN)

@perrozzi
Copy link
Contributor Author

@Dr15Jones what shall we do?

@mseidel42
Copy link
Contributor

@amarini Ok, thank you, it works like this:

process.genParticles.src = 'source:generator'
process.VtxSmeared.src = 'source:generator'

with the usual generation_step

process.generation_step = cms.Path(process.pgen)

@civanch
Copy link
Contributor

civanch commented Jun 1, 2017

+1
fine for sim

@Dr15Jones
Copy link
Contributor

+1

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_1_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_9_2_X is complete. This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @smuzaffar

@davidlange6
Copy link
Contributor

otherwise this needs a master branch PR and test.

@perrozzi
Copy link
Contributor Author

@amarini can you remind me if this was needed only in 71X?

@perrozzi
Copy link
Contributor Author

perrozzi commented Jul 3, 2017

@amarini ping

@amarini
Copy link
Contributor

amarini commented Jul 3, 2017

I think this is needed as well for newer versions, but for porting rebase is not enough, because some Input Tags in the reconstruction changed.
E.g. here-master the last string -> InputTag while here-71 it is a class member.
While the basic idea should stay the same.

@amarini amarini mentioned this pull request Jul 3, 2017
@amarini
Copy link
Contributor

amarini commented Jul 3, 2017

Port to 9x in #19523

@davidlange6
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 629714f into cms-sw:CMSSW_7_1_X Jul 19, 2017
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.

None yet

7 participants