-
Notifications
You must be signed in to change notification settings - Fork 32
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
simplify and fix optuna-hpo notebook #404
Conversation
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
*.csv | ||
*.csv.gz | ||
*.csv.zip | ||
*.tar.gz | ||
*.zip |
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.
The rapids-optuna-hpo
example ends up downloading a .zip
file, which contains some inner zipped CSV files.
That's why I added .zip
and .csv.zip
here, and then though "might as well as some other common archives too", as I don't think we ever want any of these in source control.
"metadata": {}, | ||
"outputs": [], | ||
"source": [ | ||
"## Run this cell to install optuna\n", | ||
"# !pip install optuna" | ||
"#!pip install optuna optuna-integration" |
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.
This notebook calls the following Python code:
storage = optuna.integration.DaskStorage()
The optuna.integration
module is provided by a separate package, optuna-integration
, which is not pulled in by default by pip install optuna
.
Adding this avoids a ModuleNotFoundError
later on in the notebook.
"Best params: {'C': 1.486491072441749, 'penalty': 'l2', 'fit_intercept': True}\n", | ||
"Number of finished trials: 144\n", | ||
"```" | ||
] |
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.
The logs generated above:
- are huge
- contain stacktraces with absolute paths from wherever the code was run
Proposing here to instead clear the output of that cell and just keep this shorter snippet of the logs in a markdown block.
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 thanks @jameslamb
Contributes to #402 .
I ran through
source/examples/rapids-optuna-hpo/notebook.ipynb
today. This proposes a few changes to that notebook based on that experience.Added bonus... I tested this with Python 3.11 (for #377).
Notes for Reviewers
I've left comments inline explaining the changes.