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.

Sign up for GitHub

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

Jump to bottom

[WIP] Enable WML Training #16

Merged
merged 31 commits into from
Sep 17, 2019
Merged

[WIP] Enable WML Training #16

merged 31 commits into from
Sep 17, 2019

Conversation

splovyt
Copy link
Contributor

@splovyt splovyt commented Jul 12, 2019 •
edited
Loading

This PR enables WML training functionality.

  • Restructured the original assets and uploaded them to max-assets-prod
  • Updated the Dockerfile for training
  • Bump the API version to 2.0.0
  • Added Train section to main README
  • Added the cookiecutter training and setup scripts
  • Added the model-specific training scripts

TODO:

  • Optimize code
  • Update data README.md
  • Update training README.md

@splovyt splovyt changed the title Enable WML Training [WIP] Enable WML Training Jul 12, 2019
@splovyt splovyt requested review from SSaishruthi and ptitzler July 14, 2019 22:33
@splovyt
Copy link
Contributor Author

splovyt commented Jul 14, 2019

@ptitzler @SSaishruthi This PR is ready for review now.

@splovyt
Copy link
Contributor Author

splovyt commented Jul 17, 2019

This build also succeeds with my jenkins pipeline :-)

ptitzler
ptitzler previously requested changes Jul 26, 2019
View reviewed changes
Copy link
Contributor

@ptitzler ptitzler left a comment

Choose a reason for hiding this comment

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

Still testing.

MLnick
MLnick reviewed Jul 26, 2019
View reviewed changes
Copy link
Contributor

@MLnick MLnick left a comment

Choose a reason for hiding this comment

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

First pass review comments

@splovyt
Copy link
Contributor Author

splovyt commented Jul 30, 2019

I have made both your changes and both Jenkins and Travis are succeeding.

@splovyt splovyt force-pushed the max-training branch 3 times, most recently from 3b08ef9 to 7558255 Compare August 13, 2019 17:21
MLnick
MLnick reviewed Sep 5, 2019
View reviewed changes
@MLnick
Copy link
Contributor

MLnick commented Sep 5, 2019

A couple minor comments, otherwise looks ready

bdwyer2
bdwyer2 reviewed Sep 5, 2019
View reviewed changes
bdwyer2
bdwyer2 reviewed Sep 5, 2019
View reviewed changes
@splovyt
Copy link
Contributor Author

splovyt commented Sep 6, 2019

Thanks all, have updated this!

MLnick
MLnick previously approved these changes Sep 12, 2019
View reviewed changes
Copy link
Contributor

@MLnick MLnick left a comment

Choose a reason for hiding this comment

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

LGTM

@MLnick
Copy link
Contributor

MLnick commented Sep 12, 2019

@splovyt branch needs to be updated to merge in latest master

@MLnick MLnick merged commit 3c0f003 into IBM:master Sep 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bdwyer2 bdwyer2 bdwyer2 left review comments

@MLnick MLnick MLnick approved these changes

@ptitzler ptitzler ptitzler left review comments

@SSaishruthi SSaishruthi Awaiting requested review from SSaishruthi

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@splovyt @MLnick @ptitzler @bdwyer2 @SSaishruthi