Refactor installation guide for 1.0 release#2003
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughReplaces and reorganizes installation documentation: streamlines main installation to PyPI, removes in-source install instructions, and expands the development guide with two source/dev installation methods (uv and pip-in-venv), OS-specific steps, editable installs, and updated CUDA/torch extras guidance. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
docs/guide/installation.rst (1)
100-102: Consider clarifying the nvidia-smi guidance.The current wording "you can find out your CUDA version by running
nvidia-smi" is slightly imprecise. Thenvidia-smicommand shows the CUDA Driver API version, not the toolkit version. However, for the purpose of selecting PyTorch wheels, this is exactly what's needed since the driver version determines which CUDA runtimes are compatible.Consider making this more explicit:
Run an example that runs RL policy inference. Choose the extra matching your CUDA driver version (``torch-cu12`` for CUDA 12.x, ``torch-cu13`` for CUDA 13.x); check your driver version with ``nvidia-smi`` (look for "CUDA Version" in the output):🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/guide/installation.rst` around lines 100 - 102, Update the sentence starting "Run an example that runs RL policy inference." to clarify that users should check their CUDA driver version (not toolkit) and to look for "CUDA Version" in the output of `nvidia-smi`; specifically, change the phrase "you can find out your CUDA version by running `nvidia-smi`" to indicate it reports the CUDA driver version and instruct readers to "check your driver version with `nvidia-smi` (look for 'CUDA Version' in the output)" while keeping the guidance about choosing the torch-cu12/torch-cu13 extras intact.docs/guide/development.rst (1)
19-22: Consider adding HTTPS clone option for accessibility.The git clone command uses SSH format, which requires users to have SSH keys configured with GitHub. For broader accessibility, consider adding an HTTPS alternative:
.. code-block:: console # SSH (requires configured SSH keys) git clone git@github.com:newton-physics/newton.git # or HTTPS git clone https://github.com/newton-physics/newton.git cd newton🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/guide/development.rst` around lines 19 - 22, Update the README code block that currently shows the SSH clone command "git clone git@github.com:newton-physics/newton.git" to also present an HTTPS alternative; add a short comment label for each option (e.g., "# SSH (requires configured SSH keys)" and "# or HTTPS") and include the HTTPS clone command "git clone https://github.com/newton-physics/newton.git" immediately alongside the existing SSH line so readers can choose either method before the "cd newton" line.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/guide/development.rst`:
- Around line 19-22: Update the README code block that currently shows the SSH
clone command "git clone git@github.com:newton-physics/newton.git" to also
present an HTTPS alternative; add a short comment label for each option (e.g.,
"# SSH (requires configured SSH keys)" and "# or HTTPS") and include the HTTPS
clone command "git clone https://github.com/newton-physics/newton.git"
immediately alongside the existing SSH line so readers can choose either method
before the "cd newton" line.
In `@docs/guide/installation.rst`:
- Around line 100-102: Update the sentence starting "Run an example that runs RL
policy inference." to clarify that users should check their CUDA driver version
(not toolkit) and to look for "CUDA Version" in the output of `nvidia-smi`;
specifically, change the phrase "you can find out your CUDA version by running
`nvidia-smi`" to indicate it reports the CUDA driver version and instruct
readers to "check your driver version with `nvidia-smi` (look for 'CUDA Version'
in the output)" while keeping the guidance about choosing the
torch-cu12/torch-cu13 extras intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 0a5d7b3e-df7b-4f16-a30c-7406234693ef
📒 Files selected for processing (2)
docs/guide/development.rstdocs/guide/installation.rst
adenzler-nvidia
left a comment
There was a problem hiding this comment.
Nice cleanup overall — the split between end-user installation and development guide is a clear improvement, and the extras table corrections (e.g. torch-cu12 now correctly says "includes examples") are welcome. A few items to address:
- Clarify nvidia-smi reports driver CUDA support, not toolkit version - Remove duplicate basic_pendulum run from install snippet - Switch from -f to --extra-index-url for NVIDIA pip index
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/guide/development.rst`:
- Around line 16-22: Replace or augment the SSH-only clone example "git clone
git@github.com:newton-physics/newton.git" in the docs/guide/development.rst code
block with an HTTPS alternative (e.g., "git clone
https://github.com/newton-physics/newton.git") or show both forms side-by-side
so contributors without SSH keys can clone; keep the subsequent "cd newton" step
unchanged.
- Around line 95-99: Update the four occurrences of unquoted editable extras
specifiers so shells don't glob them: replace instances of .[dev],
.[dev,torch-cu12], and .[docs] with quoted forms ".[dev]", ".[dev,torch-cu12]",
and ".[docs]" respectively in the development guide (the pip install commands
around the earlier pip install -e .[dev] and python -m pip install -e .[dev,...]
usages).
In `@docs/guide/installation.rst`:
- Around line 176-178: The docs currently link the extras to the moving `main`
branch's `pyproject.toml`; update the sentence referencing `pyproject.toml` so
it either (a) links to a version- or tag-specific `pyproject.toml` for the
release (replace the `main` URL with the release tag URL) or (b) remove the
external link and embed an authoritative table of optional dependency sets
directly in this page; update the text that mentions `pyproject.toml` to reflect
the chosen approach and ensure the link points to the tag-specific file or that
the embedded table lists the extras.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 5402cd7f-04dd-4d9f-a463-ab88a1cae191
📒 Files selected for processing (2)
docs/guide/development.rstdocs/guide/installation.rst
- Switch git clone to HTTPS - Quote pip extras specifiers for zsh compatibility - Remove unversioned pyproject.toml link
adenzler-nvidia
left a comment
There was a problem hiding this comment.
Looks good — all previous review comments have been addressed.
A couple of optional suggestions for polish:
-
installation.rst:118 — Quick Start mentions installing with the
simextra, but the installation section above no longer showspip install "newton[sim]"(removed in this PR). Consider dropping the `or ``sim``` mention for consistency. -
development.rst:167 — "following your CUDA version" could say "matching your NVIDIA driver's CUDA support" for consistency with the corrected wording in installation.rst.
Neither is blocking.
- Match nvidia-smi CUDA wording in development guide to installation guide - Drop extras mention from Quick Start intro
Description
Refactors the installation and development guides in preparation for the 1.0 release.
newton-actuators)torch-cu12vstorch-cu13selection vianvidia-smiChecklist
CHANGELOG.mdhas been updated (if user-facing change)Test plan
Built docs locally with:
Build succeeded with no warnings related to these changes.
Summary by CodeRabbit