Adapt notebooks to mlpack 4 API #208
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
|
|
zoq
reviewed
Oct 19, 2022
| ] | ||
| }, | ||
| { | ||
| "cell_type": "code", | ||
| "execution_count": 22, | ||
| "id": "97fdc354-57d0-4e28-91e8-1d707fc24226", | ||
| "metadata": {}, | ||
| "outputs": [], | ||
| "outputs": [ |
There was a problem hiding this comment.
Yeah. I suppose we could increase the penalty to work around the issue, but I don't think it's a big deal either way.
There was a problem hiding this comment.
The results are pretty much the same, so I don't think this is needed.
zoq
approved these changes
Oct 20, 2022
![mlpack-bot[bot]](https://avatars.githubusercontent.com/in/23934?s=60&v=4){kind=small}
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I went through all of the C++ notebooks and made changes that reflect the latest changes in mlpack master (which will be mlpack 4). Basically these changes are:
<mlpack.hpp>. (Adapt mlpack to require only one include; revamp docs to Markdown, not Doxygen mlpack#3265)I also made a few fixes where necessary, and sometimes I reduced the computational complexity of the models we were learning so we would stay within resource limits.
Now, every notebook runs successfully on Binder, with two exceptions:
cifar10_evalnotebook needs an 8GB binder instance to succeed. I had one of those once, when starting a notebook on Binder, but then I started getting 2GB instances. However, it should work.After merging this PR, https://datasets.mlpack.org/cifarNet2.xml will need to be moved to https://datasets.mlpack.org/cifarNet.xml.
There is one additional issue: datasets.mlpack.org is not available from OVH binder instances. I'm trying to work with the Binder Gitter chat and Github repository to get a resolution there.