-
Notifications
You must be signed in to change notification settings - Fork 861
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
New AutoGluon Webpage #2924
New AutoGluon Webpage #2924
Conversation
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
Job PR-2924-7d38808 is done. |
Job PR-2924-dce8df7 is done. |
a9176a4
to
c0fc7f8
Compare
Job PR-2924-c0fc7f8 is done. |
Job PR-2924-2e2e60a is done. |
Job PR-2924-a5fb2a6 is done. |
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.
Awesome work! Added comments.
In general, Tabular LGTM!
docs/install.md
Outdated
Modules and optional dependencies: | ||
|
||
1. `autogluon.tabular`: functionality for tabular data (TabularPredictor) | ||
- Optional dependencies included by default: `lightgbm`,`catboost`,`xgboost`,`fastai`. |
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.
These are part of the optional dependency all
, which is included by default during pip install autogluon
.
Calling pip install autogluon.tabular
will not install these dependencies. The user will need to specify pip install autogluon.tabular[all]
.
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.
I updated the modularized install instructions on my most recent push.
docs/install.md
Outdated
|
||
1. `autogluon.tabular`: functionality for tabular data (TabularPredictor) | ||
- Optional dependencies included by default: `lightgbm`,`catboost`,`xgboost`,`fastai`. | ||
- Optional dependencies not included by default: `vowpalwabbit`, `skex`. The later will speedup KNN training and inference on CPU by 25x. |
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.
later -> latter
Missing mention of optional dependencies imodels
and skl2onnx
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.
I updated the modularized install instructions on my most recent push.
@@ -0,0 +1,345 @@ | |||
{ |
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 quick start is pretty good. We may want to consider using titanic instead of this example (although we definitely should keep this example for at least 1 tutorial, since it is quite good). For the purposes of an initial merge, I am ok with this tutorial.
The main thing we should add to this tutorial is a comparison of how simple this solution is + how strong the result is compared to DeepMind's original solution that was on the cover of nature (300+ lines of code & 80% accuracy with DeepMind vs 3 lines of code & 95% accuracy with AutoGluon). This comparison shouldn't be present in a quick-start tutorial, which is why we should wait to add that comparison until we make a different quick-start tutorial for Tabular.
Job PR-2924-3a58579 is done. |
Job PR-2924-185d50f is done. |
Job PR-2924-1fe92ce is done. |
Job PR-2924-9226be5 is done. |
Job PR-2924-8cf5222 is done. |
Job PR-2924-f5541d5 is done. |
Job PR-2924-4761f3d is done. |
Job PR-2924-024484c is done. |
Job PR-2924-29f5bc5 is done. |
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 in general regarding the workflow change. Had one minor comment
sed -i -e "s@###_PLACEHOLDER_WEB_CONTENT_ROOT_###@http://$site@g" docs/config.ini | ||
sed -i -e "s@###_OTHER_VERSIONS_DOCUMENTATION_LABEL_###@$other_doc_version_text@g" docs/config.ini | ||
sed -i -e "s@###_OTHER_VERSIONS_DOCUMENTATION_BRANCH_###@$other_doc_version_branch@g" docs/config.ini |
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.
With the new website how do we generate link to stable/dev one dynamically?
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.
These ad-hoc replacement strings were used in the nav-bar of the old website to link to things like versions, api, tutorials, etc. These exact links don't exist on the new webpage, but their equivalent sections are handled by the sphinx build process automatically. I believe these only needed to be done this way because the links were hard coded in the config file, not built by sphinx.
Job PR-2924-f44ebae is done. |
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 in general. Thanks for the change and the new website looks great! Some minor comments regarding installation guide and build process
|
||
Download the PDF version with clickable links [multimodal-cheatsheet]. | ||
|
||
## Time Series |
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.
Wondering if we have the cheatsheet to be added for eda?
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.
An EDA Cheat Sheet did not exist last time I synced with Alex. I'm not sure if Alex is planning on making one in the future.
# Uninstall libomp if it was previous installed | ||
brew uninstall -f libomp | ||
wget https://raw.githubusercontent.com/Homebrew/homebrew-core/fb8323f2b170bd4ae97e1bac9bf3e2983af3fdb0/Formula/libomp.rb | ||
brew install libomp.rb |
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.
We need to add rm libomp.rb
after installation
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.
Good catch, I will add in next push
# Install the proper version of PyTorch following https://pytorch.org/get-started/locally/ | ||
pip install torch==1.13.1+cu116 torchvision==0.14.1+cu116 --extra-index-url https://download.pytorch.org/whl/cu116 |
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.
I thought we didn't need to manually install torch for GPU? Shouldn't it install automatically via pip install autogluon
?
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.
My intention was for these to be the exact same as they are on the existing website. I am not sure if your statement about not needing to manually install torch for GPU is correct but can look into it later.
# Install the proper version of PyTorch following https://pytorch.org/get-started/locally/ | ||
pip install torch==1.13.1+cu116 torchvision==0.14.1+cu116 --extra-index-url https://download.pytorch.org/whl/cu116 |
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.
I thought we didn't need to manually install torch for GPU? Shouldn't it install automatically?
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.
See above comment
|
||
:::{dropdown} M1 and M2 Apple Silicon | ||
|
||
Apple Silicon is now supported via the `conda` installation instructions outlined above. `conda-forge` will install the GPU version if a user's machine supports it. |
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.
@gradientsky please confirm if this is correct
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.
In general LGTM! We can make smaller PRs for additional improvements & follow-ups after this is merged. Amazing work!
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!
Job PR-2924-ad09d55 is done. |
Job PR-2924-79b4e81 is done. |
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.
Looks great!! Added some minor comments
Job PR-2924-0ff3285 is done. |
Job PR-2924-0ff3285 is done. |
Co-authored-by: Oleksandr Shchur <shchuro@amazon.com>
Description of changes:
This change is a major facelift on the AutoGluon webpage. It allows AutoGluon to leave behind the outdated version of d2l-book that the webpage was tied to, along with limitations on the version of Sphinx that could be used. We now use the sphinx furo theme, but can be more easily changed in the future by choosing other sphinx-design themes.
This change touches most files in the
docs
directory. I intend to split the review of this PR across:.github
directorytutorials
directory which can be inspected by different vertical teamsTutorials are now maintained in
ipynb
notebooks rather thanmd
files. This change come with significant pros and cons:Pros:
Cons:
ipynb
files is more difficult (using this tool can help mitigate this)When reviewing these changes, it is probably easier to inspect tutorials on the new webpage than it will be to look through raw files and diffs.
This PR is to merge the new website onto master/dev. Once we are comfortable with it, we will merge onto stable as well.
This checklist of items needs to be done before merge:
new
branch url from Colab/Studio Lab linkspre-
from correct locations in Install pageBy submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.