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

Allow different lhe headers in one job #2859

Merged
merged 1 commit into from
Mar 17, 2014

Conversation

vciulli
Copy link
Contributor

@vciulli vciulli commented Mar 13, 2014

Creates a new run when the lhe header differs from that of the previous lhe file

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @vciulli (Vitaliano Ciulli) for CMSSW_7_1_X.

Allow different lhe headers in one job

It involves the following packages:

FWCore/Sources
GeneratorInterface/LHEInterface
SimDataFormats/GeneratorProducts

@vciulli, @Dr15Jones, @thuer, @cmsbuild, @nclopezo, @bendavid, @Degano, @ktf can you please review it and eventually sign? Thanks.
@wmtan this is something you requested to watch as well.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.
@nclopezo, @ktf you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

@vciulli
Copy link
Contributor Author

vciulli commented Mar 14, 2014

+1

On 14 Mar 2014, at 00:53, cmsbuild notifications@github.com wrote:

A new Pull Request was created by @vciulli (Vitaliano Ciulli) for CMSSW_7_1_X.

Allow different lhe headers in one job

It involves the following packages:

FWCore/Sources
GeneratorInterface/LHEInterface
SimDataFormats/GeneratorProducts

@vciulli, @Dr15Jones, @thuer, @cmsbuild, @nclopezo, @bendavid, @Degano, @ktf can you please review it and eventually sign? Thanks.
@wmtan this is something you requested to watch as well.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.
@nclopezo, @ktf you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.


Reply to this email directly or view it on GitHub.

@Dr15Jones
Copy link
Contributor

@wmtan was the change to the framework already part of another pull request? If so, it would have been better to keep is as a separate commit rather than merging it with some LHE changes.

@Dr15Jones
Copy link
Contributor

#2834 contains part of one of the commits in this pull request. @ktf will git have a problem merging?

@wmtan
Copy link
Contributor

wmtan commented Mar 14, 2014

Chris,

@wmtan
Copy link
Contributor

wmtan commented Mar 14, 2014

Chris,
Yes the framework part was done as a separate pull request, which, I believe was already merged.
My apologies for not doing a separate commit for it in my branch.

@ktf
Copy link
Contributor

ktf commented Mar 14, 2014

If the commits in question are in the same order, it should not care. If
they are not, your milage might vary. A rebase should fix the problem in
any case.

On 14 Mar 2014, at 1:33, Chris Jones wrote:

#2834 contains part of one of the commits in this pull request. @ktf
will git have a problem merging?


Reply to this email directly or view it on GitHub:
#2859 (comment)

@cmsbuild
Copy link
Contributor

@Dr15Jones
Copy link
Contributor

After consulting with Giulio, we believe that this pull request will conflict with #2834 because the changes were not kept as seperate commits. Please rebase this code on top of the present CMSSW_7_1_X.

@wmtan
Copy link
Contributor

wmtan commented Mar 14, 2014

Vitaliano, I will do the rebasing later this morning (Fermi time), so you don't have to.

@wmtan
Copy link
Contributor

wmtan commented Mar 14, 2014

This has been rebased. Should merge now.

@cmsbuild
Copy link
Contributor

Pull request #2859 was updated. @vciulli, @nclopezo, @thuer, @cmsbuild, @bendavid, @Degano can you please check and sign again.

@vciulli
Copy link
Contributor Author

vciulli commented Mar 16, 2014

-1
I tried to merge on top of CMSSW_7_1_X_2014-03-14-0200 with SCRAM_ARCH=slc6_amd64_gcc481
and there are hundreds of different files

Am I doing something wrong or the rebase did not work correctly?

@wmtan
Copy link
Contributor

wmtan commented Mar 17, 2014

I also tried the merge, and I also see hundreds of different files. I don't understand this at all, as the commit shows only 5 files changed.
If I were you, I would create your own branch and pull request.
On the other hand, if you want me to do anything about this, it will have to wait until tomorrow (Monday),
Bill
.

@vciulli
Copy link
Contributor Author

vciulli commented Mar 17, 2014

@wmtan
I made a new pull importing your commit in the last IB
#2887

@vciulli
Copy link
Contributor Author

vciulli commented Mar 17, 2014

@wmtan
Nope. Same problem. The commit is somewhat corrupted (I cherry-picked it).
I will just copy by hand the modified files in a new branch

@vciulli
Copy link
Contributor Author

vciulli commented Mar 17, 2014

+1
I think I understood the problem: many packages depend on SimDataFormats/GeneratorProducts; git cms-merge-topic detects it and downloads all of them.
Testing that this merge builds fine may need recompiling most of CMSSW.
I'll try that on an lxbuild machine, but given that everything works fine if I cherry-pick the commit I think it is safe to approve it

@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 unless changes or unless it breaks tests. @nclopezo, @ktf can you please take care of it?

@cmsbuild
Copy link
Contributor

@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 unless changes (tests are also fine). @nclopezo, @ktf can you please take care of it?

@cmsbuild
Copy link
Contributor

ktf added a commit that referenced this pull request Mar 17, 2014
Generators -- Allow different lhe headers in one job
@ktf ktf merged commit f692482 into cms-sw:CMSSW_7_1_X Mar 17, 2014
@wmtan wmtan deleted the AllowDifferentLHEHeadersInOneJob branch May 9, 2014 16:57
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

5 participants