MNT: Add cuda12 extra, update URLs to Project-MONAI, fix capitalization#38
MNT: Add cuda12 extra, update URLs to Project-MONAI, fix capitalization#38aylward wants to merge 3 commits intoProject-MONAI:mainfrom
Conversation
Adds a cuda12 optional extra (cupy-cuda12x, pytorch cu128) so users on CUDA 12 systems can install without source-building cupy. Makes CUDA 13 the default install. Updates all GitHub URLs from aylward/PhysioMotion4d to Project-MONAI/physiomotion4d and fixes the lone PhysioMotion4d capitalisation violation in docs/conf.py.
WalkthroughUpdates docs and packaging to make CUDA 13 the default (CUDA 12 available via a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 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. Additional details and impacted files@@ Coverage Diff @@
## main #38 +/- ##
=======================================
Coverage ? 13.67%
=======================================
Files ? 49
Lines ? 6596
Branches ? 0
=======================================
Hits ? 902
Misses ? 5694
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR updates installation packaging/docs to support CUDA 12 via an optional extra while making CUDA 13 the default, and migrates repository/documentation links to the Project-MONAI/physiomotion4d GitHub org/repo (plus a capitalization fix in Sphinx config).
Changes:
- Add a
cuda12optional extra and uv PyTorch indexes/sources for CUDA-specific wheels. - Update README + docs to describe CUDA 13 default installs, CUDA 12 installs via
[cuda12], and add troubleshooting guidance. - Replace legacy GitHub URLs (
aylward/PhysioMotion4d) withProject-MONAI/physiomotion4dacross docs and project metadata.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Updates install commands (uv) and CUDA 13/12 guidance. |
| pyproject.toml | Adds cuda12 extra + uv indexes/sources; updates project URLs; removes jupyterlab from deps. |
| docs/troubleshooting.rst | Adds CUDA version mismatch troubleshooting + updates Issues link. |
| docs/references.rst | Updates citation URL to new GitHub repo. |
| docs/quickstart.rst | Updates CUDA prerequisite wording and points to [cuda12]. |
| docs/installation.rst | Updates CUDA requirements, install commands, and clone URL. |
| docs/index.rst | Updates badges/links to new GitHub repo/org. |
| docs/faq.rst | Updates GPU/CUDA guidance and adds [cuda12] explanation. |
| docs/contributing.rst | Updates repository link. |
| docs/conf.py | Updates Sphinx html_context GitHub user/repo. |
| docs/changelog.rst | Updates changelog link to new GitHub repo. |
Comments suppressed due to low confidence (2)
README.md:47
- This section switches the primary install instructions to
uv, but still recommendspip install physiomotion4d[nim]. Sincepipwon’t use the[tool.uv.sources]PyTorch CUDA indexes, users may end up with a CPU-only PyTorch wheel (andtorch.cuda.is_available()false). Recommend usinguv pip install physiomotion4d[nim]here too, or explicitly document thatpipis unsupported for GPU installs.
For development with NVIDIA NIM cloud services:
```bash
pip install physiomotion4d[nim]
**docs/installation.rst:55**
* This page recommends `pip install physiomotion4d[nim]`, but the rest of the install guidance depends on `uv` to pull CUDA-enabled PyTorch from the configured `tool.uv.index` entries. Consider switching this to `uv pip install physiomotion4d[nim]` (or warning that `pip` installs may pull CPU-only PyTorch and break GPU functionality).
For development with NVIDIA NIM cloud services:
.. code-block:: bash
pip install physiomotion4d[nim]
</details>
---
💡 <a href="/Project-MONAI/physiomotion4d/new/main?filename=.github/instructions/*.instructions.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Add Copilot custom instructions</a> for smarter, more guided reviews. <a href="https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Learn how to get started</a>.
| # Utilities | ||
| "matplotlib>=3.5.0", | ||
| "jupyterlab>=4.0.0", | ||
| "typing-extensions>=4.0.0", | ||
| "cupy-cuda13x>=13.6.0", | ||
|
|
There was a problem hiding this comment.
cupy-cuda13x is still a hard dependency. Installing physiomotion4d[cuda12] will therefore install both cupy-cuda13x and cupy-cuda12x, which defeats the purpose of the CUDA 12 extra and can cause conflicting cupy installs. Consider moving CuPy into mutually exclusive optional extras (e.g., cuda13 and cuda12) or otherwise making the CUDA 13 CuPy dependency conditional so only one CuPy CUDA wheel is installed.
| [project.optional-dependencies] | ||
| cuda12 = [ | ||
| "cupy-cuda12x>=12.0.0", | ||
| ] |
There was a problem hiding this comment.
The cuda12 optional extra only adds cupy-cuda12x but does not prevent the base cupy-cuda13x requirement from being installed. If the intent is to let CUDA 12 users install instead of CUDA 13 CuPy wheels, restructure dependencies so selecting cuda12 results in a single CuPy distribution being installed (and update docs accordingly).
docs/installation.rst
Outdated
|
|
||
| git clone https://github.com/aylward/PhysioMotion4d.git | ||
| git clone https://github.com/Project-MONAI/physiomotion4d.git | ||
| cd PhysioMotion4D |
There was a problem hiding this comment.
git clone https://github.com/Project-MONAI/physiomotion4d.git will create a physiomotion4d/ directory by default, but the next line says cd PhysioMotion4D. Either change the cd command to match the cloned directory name or add an explicit destination to git clone so the instructions work as written.
| cd PhysioMotion4D | |
| cd physiomotion4d |
|
|
||
| # CUDA 12 | ||
| uv pip install "physiomotion4d[cuda12]" | ||
|
|
There was a problem hiding this comment.
These instructions rely on uv pip to select the correct PyTorch CUDA index, but later in this same doc the suggested fix for import errors is pip install --upgrade physiomotion4d, which can silently replace CUDA-enabled PyTorch with a CPU wheel. Consider using uv pip consistently in troubleshooting commands (or clearly warn against using plain pip for installs/upgrades).
| .. warning:: | |
| Use ``uv pip`` consistently for installing **and upgrading** ``physiomotion4d`` so that the | |
| correct CUDA-enabled PyTorch and related wheels are selected. Using plain ``pip`` | |
| (especially without ``--no-deps``) may reinstall a CPU-only PyTorch wheel and break | |
| GPU support. |
| nvidia-smi # shows driver and CUDA version | ||
|
|
||
| .. note:: | ||
| CPU-only installation is not supported. A CUDA-capable NVIDIA GPU is required. | ||
|
|
There was a problem hiding this comment.
The new note says CPU-only installation is not supported, but earlier in this section you recommend switching processing to CPU to resolve CUDA OOM. Please clarify the distinction (e.g., CPU execution may be possible after a CUDA-enabled install, but installing/running on systems without NVIDIA/CUDA is unsupported) to avoid contradictory guidance.
docs/faq.rst
Outdated
| CUDA 13 is the default. If your system has CUDA 12, install the ``[cuda12]`` extra | ||
| instead: | ||
|
|
||
| .. code-block:: bash | ||
|
|
||
| uv pip install "physiomotion4d[cuda12]" | ||
|
|
||
| The ``[cuda12]`` extra provides ``cupy-cuda12x>=12.0.0`` and sources PyTorch, | ||
| torchvision, and torchaudio from ``https://download.pytorch.org/whl/cu128``. | ||
| The default install provides ``cupy-cuda13x>=13.6.0``. |
There was a problem hiding this comment.
This FAQ implies the [cuda12] extra is an alternative to the default CUDA 13 install. However, cupy-cuda13x is currently a required dependency, so physiomotion4d[cuda12] will install both CUDA 13 and CUDA 12 CuPy wheels. Either adjust the packaging to make the extras mutually exclusive, or update the wording here to avoid implying a replacement.
| CUDA 13 is the default. If your system has CUDA 12, install the ``[cuda12]`` extra | |
| instead: | |
| .. code-block:: bash | |
| uv pip install "physiomotion4d[cuda12]" | |
| The ``[cuda12]`` extra provides ``cupy-cuda12x>=12.0.0`` and sources PyTorch, | |
| torchvision, and torchaudio from ``https://download.pytorch.org/whl/cu128``. | |
| The default install provides ``cupy-cuda13x>=13.6.0``. | |
| CUDA 13 is the default and ``cupy-cuda13x>=13.6.0`` is always installed. | |
| If your system has CUDA 12 and you also need CUDA 12 support, you can add the | |
| ``[cuda12]`` extra: | |
| .. code-block:: bash | |
| uv pip install "physiomotion4d[cuda12]" | |
| The ``[cuda12]`` extra additionally provides ``cupy-cuda12x>=12.0.0`` and sources | |
| PyTorch, torchvision, and torchaudio from | |
| ``https://download.pytorch.org/whl/cu128``. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/installation.rst`:
- Around line 65-66: The README/doc step uses the wrong directory casing: after
running the git clone command shown ("git clone
https://github.com/Project-MONAI/physiomotion4d.git") the cloned folder is
"physiomotion4d", so update the install step that currently says "cd
PhysioMotion4D" to use the actual directory name "cd physiomotion4d" (match the
lowercase repo name) so the command works on case-sensitive filesystems; update
the line in docs/installation.rst accordingly.
In `@pyproject.toml`:
- Around line 117-119: Base dependencies include "cupy-cuda13x" while the extras
group "cuda12" adds "cupy-cuda12x", causing both CuPy CUDA variants to be
installed; make them mutually exclusive by removing "cupy-cuda13x" from the base
dependency list and instead put it into a new extras group (e.g., "cuda13") or
conditionally include it only when "cuda13" is selected; update the
pyproject.toml so the base dependencies no longer reference "cupy-cuda13x" and
ensure extras groups "cuda12" and the new "cuda13" contain "cupy-cuda12x" and
"cupy-cuda13x" respectively to avoid installing both variants simultaneously.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 03906716-ee26-4f4a-b94a-5eb1d1c4e1a9
📒 Files selected for processing (11)
README.mddocs/changelog.rstdocs/conf.pydocs/contributing.rstdocs/faq.rstdocs/index.rstdocs/installation.rstdocs/quickstart.rstdocs/references.rstdocs/troubleshooting.rstpyproject.toml
uv requires that any package with extra = "cuda12" in [tool.uv.sources] must also appear in that extra's [project.optional-dependencies] list.
Moves cupy-cuda13x into a cuda13 extra (symmetric with cuda12) so neither CUDA variant is silently installed by a bare pip install. Adds an import-time check in __init__.py that raises a clear error directing users to install one of the two extras. Updates all docs and README to show explicit [cuda13] install commands.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
README.md (1)
37-47: Inconsistent package manager:uv pipvspip.Lines 38 and 41 use
uv pip installfor the CUDA extras, but line 46 still uses plainpip installfor the NIM extra. This inconsistency may confuse users about which tool to use.Suggested fix for consistency
For development with NVIDIA NIM cloud services: ```bash -pip install physiomotion4d[nim] +uv pip install "physiomotion4d[nim]"</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@README.mdaround lines 37 - 47, Replace the inconsistent plain pip
invocation for the NIM extra with the same "uv pip" form used for the CUDA
extras: change the line that installs "physiomotion4d[nim]" to use uv pip
install "physiomotion4d[nim]" so all examples consistently show the uv wrapper
(look for the literal strings "physiomotion4d[nim]" and the existing "uv pip
install" examples for CUDA to locate the spot to update).</details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In@README.md:
- Around line 37-47: Replace the inconsistent plain pip invocation for the NIM
extra with the same "uv pip" form used for the CUDA extras: change the line that
installs "physiomotion4d[nim]" to use uv pip install "physiomotion4d[nim]" so
all examples consistently show the uv wrapper (look for the literal strings
"physiomotion4d[nim]" and the existing "uv pip install" examples for CUDA to
locate the spot to update).</details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: Organization UI **Review profile**: CHILL **Plan**: Pro **Run ID**: `430a728e-e75f-4155-8ce6-b25072f38b89` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between d07d2b0fb19c2dd2ad44e80c31e75bfecb16456e and a4682b255fe5dd3a89c77319551e6ec28c3e8141. </details> <details> <summary>📒 Files selected for processing (6)</summary> * `README.md` * `docs/faq.rst` * `docs/installation.rst` * `docs/troubleshooting.rst` * `pyproject.toml` * `src/physiomotion4d/__init__.py` </details> <details> <summary>✅ Files skipped from review due to trivial changes (1)</summary> * docs/troubleshooting.rst </details> <details> <summary>🚧 Files skipped from review as they are similar to previous changes (3)</summary> * docs/faq.rst * pyproject.toml * docs/installation.rst </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
docs/installation.rst:55
- This NIM installation command (
pip install physiomotion4d[nim]) does not include either CUDA extra, so it won’t install CuPy and will fail atimport physiomotion4ddue to the new import-time CuPy requirement. Consider updating it to require one of[cuda13,nim]/[cuda12,nim](and/or useuv pipto ensure the CUDA-enabled PyTorch wheels are selected).
For development with NVIDIA NIM cloud services:
.. code-block:: bash
pip install physiomotion4d[nim]
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| [project.optional-dependencies] | ||
| cuda13 = [ | ||
| "cupy-cuda13x>=13.6.0", | ||
| ] | ||
| cuda12 = [ | ||
| "cupy-cuda12x>=12.0.0", | ||
| "torch>=2.0.0,<3.0.0", | ||
| "torchvision", | ||
| "torchaudio", | ||
| ] |
There was a problem hiding this comment.
physiomotion4d now hard-requires cupy at import time (see __init__.py), but cupy-cuda13x is no longer in the base dependencies list (it’s only in the cuda13 extra). This means a plain pip install physiomotion4d / uv pip install physiomotion4d will install successfully but immediately fail on import physiomotion4d. If CUDA 13 is meant to be the default install, add cupy-cuda13x back to core dependencies (and keep cuda12 as an override), or otherwise relax/move the CuPy import check so the base install remains usable.
| " uv pip install 'physiomotion4d[cuda13]' # CUDA 13 (recommended)\n" | ||
| " uv pip install 'physiomotion4d[cuda12]' # CUDA 12\n\n" |
There was a problem hiding this comment.
The import-time error message uses single quotes around the install commands (uv pip install 'physiomotion4d[cuda13]'). Elsewhere in the docs/README the commands use double quotes; aligning to double quotes avoids shell-quoting pitfalls (notably on Windows) and keeps the guidance consistent.
| " uv pip install 'physiomotion4d[cuda13]' # CUDA 13 (recommended)\n" | |
| " uv pip install 'physiomotion4d[cuda12]' # CUDA 12\n\n" | |
| ' uv pip install "physiomotion4d[cuda13]" # CUDA 13 (recommended)\n' | |
| ' uv pip install "physiomotion4d[cuda12]" # CUDA 12\n\n' |
| # CUDA 13 install (recommended) | ||
| uv pip install "physiomotion4d[cuda13]" | ||
|
|
||
| # CUDA 12 install | ||
| uv pip install "physiomotion4d[cuda12]" |
There was a problem hiding this comment.
The CUDA install commands were updated to use uv + [cuda13]/[cuda12], but the NIM install snippet right below still uses pip install physiomotion4d[nim], which won’t pull in CuPy and will fail at import physiomotion4d given the new import-time CuPy requirement. Update the NIM instructions to include a CUDA extra as well (and preferably use uv pip consistently), e.g. [cuda13,nim] or [cuda12,nim].
Adds a cuda12 optional extra (cupy-cuda12x, pytorch cu128) so users on CUDA 12 systems can install without source-building cupy. Makes CUDA 13 the default install. Updates all GitHub URLs from aylward/PhysioMotion4d to Project-MONAI/physiomotion4d and fixes the lone PhysioMotion4d capitalisation violation in docs/conf.py.
Summary by CodeRabbit
Documentation
Chores
Behavior