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

Fixing neglected gen filters from HepMC renaming #16287

Merged
merged 3 commits into from Oct 31, 2016

Conversation

kurtejung
Copy link
Contributor

HepMC naming migration from commit (here: f719245) neglected to rename the inputs to a few gen filters needed for HI production

Also pinging @mandrenguyen @yetkinyilmaz @bendavid

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @kurtejung (Kurt Jung) for CMSSW_8_0_X.

It involves the following packages:

GeneratorInterface/GenFilters

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

cms-bot commands are listed here #13028

@govoni
Copy link
Contributor

govoni commented Oct 21, 2016

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 21, 2016

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/15870/console

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_8_0_X IBs after it passes the integration tests. This pull request requires discussion in the ORP meeting before it's merged. @slava77, @davidlange6, @smuzaffar

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

@perrozzi
Copy link
Contributor

@kurtejung please port this PR also to 81X and cross-reference it, thanks

@perrozzi
Copy link
Contributor

also, I see that you only changed 3 filters. Did you check them all? If there are others that need this fix, may I very kindly ask you to change them all in one PR? That would be a great service for the whole GEN community, thanks

@kurtejung
Copy link
Contributor Author

Most of the gen filters seem to already have this fix, e.g. https://github.com/cms-sw/cmssw/blob/CMSSW_8_0_X/GeneratorInterface/GenFilters/src/PythiaFilter.cc

Some of the filters clearly require the user to override the default settings (e.g. https://github.com/cms-sw/cmssw/blob/CMSSW_8_0_X/GeneratorInterface/GenFilters/src/MCLongLivedParticles.cc) and others use the genParticles.

It may be good to go through these filters in a more systematic way and change all the default values of those that use a HepMC input to "generator::unsmeared" but I was afraid of breaking filters. Please advise.

@kurtejung
Copy link
Contributor Author

Maybe @wmtan can describe why only some of the gen filters default collections were renamed from "generator" to "generator : unsmeared"

@perrozzi
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Pull request #16287 was updated. @cmsbuild, @govoni, @perrozzi, @thuer, @davidlange6 can you please check and sign again.

@kurtejung
Copy link
Contributor Author

All gen filters (both HI and pp) should be updated to properly use the unsmeared HepMC collections now - can one of the managers run tests? Thanks.

@perrozzi
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 24, 2016

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/15929/console

@perrozzi
Copy link
Contributor

please port this PR also to 81X and cross-reference it, thanks

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

@perrozzi
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_8_0_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @slava77, @davidlange6, @smuzaffar

@perrozzi
Copy link
Contributor

corresponding PR in 81X here #16331

@perrozzi
Copy link
Contributor

hi @davidlange6, this should have gone to 8_0_22
sorry if there has been any miscommunication.
can it be merged asap? thanks

@davidlange6
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 83b79a6 into cms-sw:CMSSW_8_0_X Oct 31, 2016
@davidlange6
Copy link
Contributor

as usual a note in the ORP is what will get your PRs merged…

On Oct 26, 2016, at 3:28 PM, perrozzi notifications@github.com wrote:

hi @davidlange6, this should have gone to 8_0_22
sorry if there has been any miscommunication.
can it be merged asap? thanks


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@mandrenguyen
Copy link
Contributor

@davidlange6 : A note in the google doc or the twiki? Can anyone add a note or is that only for those who sign?

@davidlange6
Copy link
Contributor

anyone can add a note (though indeed the idea is that the signing areas take responsibility for knowing what code in their area is needed where...

On Oct 31, 2016, at 3:52 PM, mandrenguyen notifications@github.com wrote:

@davidlange6 : A note in the google doc or the twiki? Can anyone add a note or is that only for those who sign?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

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

6 participants