-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Staging to master to add NCF hyperparameter tuning #1102
Conversation
…enders into seanytak/ncf_test_tuning
…menders into cheetm/add_lightfm_deepdive
…menders into cheetm/add_lightfm_deepdive
cheetm/lightfm_deep_dive
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks and this is really cool! I left some comments but couldn't finish the review. Will work on a bit more tomorrow!
...
Ok, done checking. It would be much cleaner if we format codes. Also I found some files are missing the license term.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm! Great work. Really appreciate to all the contributors!
For optional input port / parameters, the reference in implementation/args must be placed in a nested list to indicate that it is optional. Refer to https://aka.ms/azureml-module-optional-inputs for details. For the subparameters (the ones that under another parameter's `options` section), should also put the related args into nested lists, otherwise the module will fail to register in the latest CLI tool. This PR is to update the module spec to fix the issue.
Put sub parameters into nested arguments
Addressing comments for PR #1102 and some other fixes
Description
Related Issues
#1092
Checklist:
staging
and notmaster
.