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

Update the GeneratorFilter template class for new calling API. #17244

Merged
merged 1 commit into from Feb 7, 2017

Conversation

bbockelm
Copy link
Contributor

First round of improvements to allow multicore invocation of the generators missed this interface.

This was suggested by @bendavid but @davidlange6 merged the PR before my dev host finished compiling :/

@bendavid - the cmsDriver command you suggested failed (seems to simply be a difference between 7_1 and 9_0); have another suggested one to try?

First round of improvements to allow multicore invocation of
the generators missed this interface.
@cmsbuild
Copy link
Contributor

A new Pull Request was created by @bbockelm (Brian Bockelman) for CMSSW_9_0_X.

It involves the following packages:

FWCore/Framework
GeneratorInterface/Core

@smuzaffar, @Dr15Jones, @perrozzi, @thuer, @cmsbuild, @govoni, @davidlange6 can you please review it and eventually sign? Thanks.
@Martin-Grunewald, @agrohsje, @mkirsano, @wddgit, @wmtan this is something you requested to watch as well.
@davidlange6, @smuzaffar you are the release manager for this.

cms-bot commands are listed here #13028

@bbockelm
Copy link
Contributor Author

Alright, I poked through the relvals and ran a few. All looks good.

The generator scripts are definitely getting the number of cores passed correctly; I'm not sure if it's getting to madevent correctly, however. Eyeballing things, it looks right: it's possible that the generators I'm picking from random relval results just finish too quickly.

@bendavid
Copy link
Contributor

The code changes look ok to me, but this probably needs to be tested with a different cmsDriver command, since the ones you were using previously were specific to the externalLHEProducer case I think.

@perrozzi
Copy link
Contributor

I assume we wait for further input or shall we proceed with cmsbuil validation?

@bendavid
Copy link
Contributor

Can run the tests to make sure it doesn't break any of the standard workflows. I don't think the SUSY workflow gets explicitly tested, so indeed we'll have to do that offline (where the current status is that I ow @bbockelm a cmsDriver command to test)

@perrozzi
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jan 27, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/17465/console Started: 2017/01/27 08:09

@bbockelm
Copy link
Contributor Author

Yup! I'm happy to test (debug & fix as necessary) any cmsDriver command you can suggest.

Thanks!

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

@govoni
Copy link
Contributor

govoni commented Jan 30, 2017

dear All, were the additional tests suggested by Josh performed?

@bendavid
Copy link
Contributor

An example cmsDriver command which could be tested with this change could be from this request for example:
https://cms-pdmv.cern.ch/mcm/requests?prepid=SUS-RunIISummer16FSPremix-00007&page=0&shown=131199

@bbockelm
Copy link
Contributor Author

@bendavid - the suggested command dies with

Exception: Neither gen fragment of input files provided: this is an inconsistent GEN step configuration

Thoughts? Here's precisely what I tried:

cmsDriver.py Configuration/GenProduction/python/SUS-RunIISummer16FSPremix-00007-fragment.py --fileout file:SUS-RunIISummer16FSPremix-00007.root  --pileup_input "dbs:/Neutrino_E-10_gun/RunIISummer16FSPremix-PUMoriond17_80X_mcRun2_asymptotic_2016_TrancheIV_v4-v1/GEN-SIM-DIGI-RAW" --mc --eventcontent AODSIM --fast --customise SimGeneral/DataMixingModule/customiseForPremixingInput.customiseForPreMixingInput,Configuration/DataProcessing/Utils.addMonitoring --datatier AODSIM --conditions 80X_mcRun2_asymptotic_2016_TrancheIV_v6 --beamspot Realistic50ns13TeVCollision --customise_commands "process.source.numberEventsInLuminosityBlock = cms.untracked.uint32(200)" --step GEN,SIM,RECOBEFMIX,DIGIPREMIX_S2,DATAMIX,L1,DIGI2RAW,L1Reco,RECO,HLT:@fake1 --datamix PreMix --era Run2_2016 --python_filename SUS-RunIISummer16FSPremix-00007_1_cfg.py --no_exec -n 1444

@Dr15Jones
Copy link
Contributor

+1
The framework changes are fine (and match what was done for EDProducerBase).

@perrozzi
Copy link
Contributor

perrozzi commented Feb 3, 2017

what is the status with fastsim? shall we wait for further tests/clarifications or approve anyway, in the meanwhile?

@bbockelm
Copy link
Contributor Author

bbockelm commented Feb 3, 2017

I would suggest approve anyway - it's certainly broken without this PR and likely working with.

However, I can't really verify until we have a working cmsDriver command for this module for CMSS_9.

@perrozzi
Copy link
Contributor

perrozzi commented Feb 6, 2017

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 6, 2017

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

@davidlange6
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit f0e2dc7 into cms-sw:CMSSW_9_0_X Feb 7, 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