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

Notebook showcasing NNI on Surprise SVD #687

Merged
merged 56 commits into from
Apr 18, 2019
Merged

Notebook showcasing NNI on Surprise SVD #687

merged 56 commits into from
Apr 18, 2019

Conversation

anargyri
Copy link
Collaborator

@anargyri anargyri commented Mar 27, 2019

Description

NNI is a toolkit for hyperparameter tuning that incorporates several tuning methods from the literature.
This notebook shows how to apply NNI for tuning Surprise SVD on Movielens100K.

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/687

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

Copy link
Collaborator

@gramhagen gramhagen left a comment

Choose a reason for hiding this comment

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

'took about 10' minutes right?

@anargyri
Copy link
Collaborator Author

'took about 10' minutes right?

Fixed.

reco_utils/nni/nni_utils.py Outdated Show resolved Hide resolved
reco_utils/nni/nni_utils.py Show resolved Hide resolved
reco_utils/nni/nni_utils.py Outdated Show resolved Hide resolved
scripts/generate_conda_file.py Show resolved Hide resolved
@anargyri
Copy link
Collaborator Author

I was trying pytest-httpserver
https://pytest-httpserver.readthedocs.io/en/latest/index.html
https://pypi.org/project/pytest-httpserver/
which mocks an HTTP server, for testing the NNI utils.
What do you think? Should we have this type of tests? It would require adding pytest-httpserver to the conda env though.

@gramhagen
Copy link
Collaborator

i'd recommend just leveraging mock to patch requests and provide the desired results, we already have that in our environment.

https://stackoverflow.com/questions/15753390/how-can-i-mock-requests-and-the-response

Copy link
Collaborator

@miguelgfierro miguelgfierro left a comment

Choose a reason for hiding this comment

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

apart from some docstrings that are missing, the code looks awesome! great job @anargyri

reco_utils/nni/nni_utils.py Show resolved Hide resolved
"cell_type": "markdown",
"metadata": {},
"source": [
"Hyperband follows a different style of configuration from other tuners. See [the NNI documentation](https://nni.readthedocs.io/en/latest/hyperbandAdvisor.html). Note that the [training script](../../reco_utils/nni/svd_training.py) needs to be adjusted as well, since each Hyperband trial receives an additional parameter `STEPS`, which corresponds to the resource allocation _r<sub>i</sub>_ in the [Hyperband algorithm](https://arxiv.org/pdf/1603.06560.pdf). In this example, we used `STEPS` in combination with `R` to determine the number of epochs that SVD will run for in every trial."
Copy link

@QuanluZhang QuanluZhang Apr 17, 2019

Choose a reason for hiding this comment

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

We will change STEPS to TRIAL_BUDGET for easy understanding in the next release v0.7, very sorry for this broken change. We will inform you when v0.7 is released. Planed to be in next week (microsoft/nni#963). Thanks.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for the notice

@anargyri
Copy link
Collaborator Author

anargyri commented Apr 17, 2019

I added tests except get_trials() which is not clear how to do. It requires both the patch decorator and TemporaryDirectory, which don't seem to combine.
If there is no workaround, I could break the method in two.

@anargyri anargyri merged commit 3fa1228 into staging Apr 18, 2019
@anargyri anargyri deleted the andreas/nni branch April 18, 2019 08:47
yueguoguo pushed a commit that referenced this pull request Sep 9, 2019
Notebook showcasing NNI on Surprise SVD
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
notebook Notebook related issues tuning
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants