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

O16N lgbm criteo #682

Merged
merged 8 commits into from
Apr 5, 2019
Merged

O16N lgbm criteo #682

merged 8 commits into from
Apr 5, 2019

Conversation

miguelgfierro
Copy link
Collaborator

Description

Same code as c4ba0dc
Splitting this PR #680 into 2

Related Issues

Checklist:

  • My code follows the code style of this project, as detailed in our contribution guidelines.
  • I have added tests.
  • I have updated the documentation accordingly.

@review-notebook-app
Copy link

Check out this pull request on ReviewNB: https://app.reviewnb.com/Microsoft/Recommenders/pull/682

Visit www.reviewnb.com to know how we simplify your Jupyter Notebook workflows.

@miguelgfierro miguelgfierro mentioned this pull request Mar 26, 2019
Merged
3 tasks
yueguoguo
yueguoguo reviewed Mar 27, 2019
View reviewed changes
Copy link
Collaborator

@yueguoguo yueguoguo left a comment

Choose a reason for hiding this comment

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

Would it be good to have an architecture diagram, like what we have in the existing o16n notebook?

notebooks/05_operationalize/lightgbm_criteo_o16n.ipynb Outdated Show resolved Hide resolved
notebooks/05_operationalize/lightgbm_criteo_o16n.ipynb Show resolved Hide resolved
notebooks/05_operationalize/lightgbm_criteo_o16n.ipynb Show resolved Hide resolved
jreynolds01
jreynolds01 approved these changes Apr 4, 2019
View reviewed changes
Copy link
Collaborator

@jreynolds01 jreynolds01 left a comment

Choose a reason for hiding this comment

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

Looks good - it works, but we are still working on performance tuning, which could be an iteration on it.

@miguelgfierro
Copy link
Collaborator Author

@jreynolds01 merging then, I will update the vanity url so the notebook is linked to staging

@miguelgfierro miguelgfierro merged commit 159c4cc into staging Apr 5, 2019
@miguelgfierro miguelgfierro deleted the o16_lgbm_criteo branch April 5, 2019 13:35
@gramhagen gramhagen mentioned this pull request Apr 10, 2019
Closed
yueguoguo pushed a commit that referenced this pull request Sep 9, 2019
@miguelgfierro
Merge pull request #682 from Microsoft/o16_lgbm_criteo
8500d06 
O16N lgbm criteo
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yueguoguo yueguoguo yueguoguo left review comments

@jreynolds01 jreynolds01 jreynolds01 approved these changes

@gramhagen gramhagen Awaiting requested review from gramhagen gramhagen is a code owner

@anargyri anargyri Awaiting requested review from anargyri anargyri is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
LightGBM Based Recommender Scenario
Development

Successfully merging this pull request may close these issues.
5 participants
@miguelgfierro @yueguoguo @gramhagen @jreynolds01 @anargyri