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

Added new trainings for 80X next to old ones #4

Merged
1 commit merged into from Jan 27, 2016
Merged

Conversation

smoortga
Copy link
Contributor

Added new trainings next to old ones with new names for easier merging with cmssw PR.

This PR should replace the one that is currently pending: #2
And also the revert of the merge that happened for that PR:
#3

Now that the new trainings are next to the old ones, this PR can safely be merged without conflicting with any cmssw tests. Once this merge is complete, the CMSSW PR (cms-sw/cmssw#13054) can be merged with the new naming convention in place.

@slava77 @mverzett

@cmsbuild
Copy link

A new Pull Request was created by @smoortga for branch master.

@cmsbuild, @smuzaffar, @Degano, @iahmad-khan, @davidlange6 can you please review it and eventually sign? Thanks.
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.

external issue cms-sw/cmsdist#2095

@mverzett
Copy link
Contributor

As agreed with @slava77 we will take care of removing the now outdated files once everything is merged and working.

@slava77
Copy link

slava77 commented Jan 26, 2016

Looking at smoortga/RecoBTag-CTagging@a2d6ff7...90a017c which is what I expect is a comparison with the old version (before #2 went in and got overwritten),
I see only an addition of new files.
This is good.

https://github.com/cms-data/RecoBTag-CTagging/pull/4/files shows changes to old files as well. Is this due to some feature of the revert that didn't revert everything?
@Degano do you know?

Based on smoortga/RecoBTag-CTagging@a2d6ff7...90a017c, I think this PR can be merged.
... if the https://github.com/cms-data/RecoBTag-CTagging/pull/4/files is right and the revert of #2 in #3 actually didn't go back to the original files, then I think this PR needs to be merged ASAP.

@smoortga
Copy link
Contributor Author

I think what happened in https://github.com/cms-data/RecoBTag-CTagging/pull/4/files is the following:

I went back to my latest local commit in which I replaced the old files by the new ones keeping the names as they were. Since this was not good, I copied these new trainings to the new .xml names (with the extention "_sklearn"), and I copied the old training files own-handed again in the repository. That explains wht github thinks I changed the files without the extension, and added the ones with the extension.
So I think this PR can indeed safely be merged. I checked in an editor and all training files seem to be the correct ones.

@slava77
Copy link

slava77 commented Jan 26, 2016

@Degano @smuzaffar
please propagate these updates to the IB

ghost pushed a commit that referenced this pull request Jan 27, 2016
Added new trainings for 80X next to old ones
@ghost ghost merged commit 566ba5c into cms-data:master Jan 27, 2016
@ghost
Copy link

ghost commented Jan 27, 2016

@slava77 The CMSDIST PR is in testing here: cms-sw/cmsdist#2087

This pull request was closed.
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

4 participants