Upgrade to use CUDA 13#586
Conversation
Signed-off-by: M Q <mingmelvinq@nvidia.com>
|
Warning Review limit reached
More reviews will be available in 11 minutes and 20 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. WalkthroughThis PR migrates project references and runtime installs from CUDA 12 variants to CUDA 13: CI/readthedocs, dependency files, operator comments/docstrings, tutorial packaging commands, README, third‑party notices, and adds richer logging/diagnostics to the nvimgcodec decoder tests. ChangesCUDA 12 to 13 Migration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
Pull request overview
This PR updates the MONAI Deploy App SDK’s NVIDIA/CUDA-related dependencies and documentation to target CUDA 13, including Holoscan and nvImageCodec, and adjusts CI/setup scripts and tutorial examples accordingly.
Changes:
- Bump CUDA-specific dependencies from
cu12tocu13across requirements, notices, and operator docs. - Update CI and helper scripts to install CUDA runtime version
13.*. - Improve
nvimgcodecdecoder unit test diagnostics (timing + detailed logging) and remove--cuda 12from tutorial notebook commands.
Reviewed changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
THIRD_PARTY_NOTICES/nvidia-nvimgcodec_Apache2.0_LICENSE.txt |
Updates referenced nvimgcodec PyPI package variant to cu13. |
tests/unit/test_decoder_nvimgcodec.py |
Adds verbose console logging/timing to help diagnose decoder differences. |
setup.cfg |
Switches Holoscan dependency to holoscan-cu13. |
run |
Updates CUDA runtime installation to nvidia-cuda-runtime==13.*. |
requirements.txt |
Switches Holoscan dependency to holoscan-cu13. |
requirements-examples.txt |
Updates nvimgcodec example dependency to cu13. |
requirements-dev.txt |
Updates nvimgcodec dev dependency to cu13. |
README.md |
Updates prerequisites for CUDA 13 / Holoscan CUDA 13 and related links/text. |
notebooks/tutorials/01_simple_app.ipynb |
Removes --cuda 12 from packaging command. |
notebooks/tutorials/02_mednist_app.ipynb |
Removes --cuda 12 from packaging command. |
notebooks/tutorials/02_mednist_app-prebuilt.ipynb |
Removes --cuda 12 from packaging command. |
notebooks/tutorials/03_segmentation_app.ipynb |
Removes --cuda 12 from packaging command. |
notebooks/tutorials/04_monai_bundle_app.ipynb |
Removes --cuda 12 from packaging command. |
notebooks/tutorials/05_multi_model_app.ipynb |
Removes --cuda 12 from packaging command. |
monai/deploy/operators/dicom_series_to_volume_operator.py |
Updates docstring dependency guidance to CUDA 13 nvimgcodec package. |
monai/deploy/operators/decoder_nvimgcodec.py |
Updates dependency strings to refer to CUDA 13 nvjpeg2k package. |
.github/workflows/pr.yml |
Updates CI to install CUDA runtime version 13.*. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
setup.cfg (1)
27-27: 💤 Low valueConsider adding version constraints consistent with
requirements.txt.The
requirements.txtfile specifiesholoscan-cu13>=4.0.0,<4.3.0, butsetup.cfghas no version constraint. While this might be intentional for flexibility, consider whether the same constraints should apply to ensure consistent installations across different installation methods.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@setup.cfg` at line 27, The setup.cfg entry for the package "holoscan-cu13" lacks version constraints; update the package specification in setup.cfg (the install_requires or options.install_requires list where "holoscan-cu13" appears) to match requirements.txt by using "holoscan-cu13>=4.0.0,<4.3.0" so installations via setup.cfg/packaging use the same bounds as pip installs.README.md (1)
41-41: ⚡ Quick winClarify version compatibility statement.
The documentation states "holoscan-cu13 and holoscan-cli up to version 4.2 are compatible," but the version constraint in
requirements.txtis>=4.0.0,<4.3.0, which allows any 4.2.x version. Please clarify:
- Does "up to version 4.2" mean "up to and including all 4.2.x versions" (matching the
<4.3.0constraint)?- Or does it mean "up to version 4.2.0 exactly"?
If the former, consider rewording to "version 4.2.x" or "versions 4.0 through 4.2" for clarity.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` at line 41, The README's compatibility sentence is ambiguous about whether "up to version 4.2" includes all 4.2.x releases while requirements.txt uses ">=4.0.0,<4.3.0"; update the README to explicitly state the intended range (e.g., "compatible with holoscan-cli versions 4.0 through 4.2 (including 4.2.x)") so it matches the requirements.txt constraint, referencing the package names holoscan-cu13 and holoscan-cli and the version constraint expression >=4.0.0,<4.3.0 to ensure clarity.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/pr.yml:
- Line 26: Replace the floating pip install spec "nvidia-cuda-runtime==13.*"
with a pinned patch-level version (for example "nvidia-cuda-runtime==13.3.29")
in all occurrences; search for the exact install command string and update each
instance so the same single patch version is used consistently across the
workflow steps to ensure deterministic CI.
In `@notebooks/tutorials/01_simple_app.ipynb`:
- Line 962: The packaging command string "!monai-deploy package
simple_imaging_app -c simple_imaging_app/app.yaml -t {tag_prefix}:1.0 --platform
x86_64 -l DEBUG" omits CUDA details making the base image implicit; update the
notebook to either (a) pin the CUDA base image explicitly by adding the
appropriate --cuda option/value to the monai-deploy package command (e.g., set
it to nvcr.io/nvidia/cuda:12.6.0-runtime-ubuntu22.04 if the CLI supports a
--cuda <image> flag) or (b) add a one-line note immediately adjacent to that
command string stating the exact CUDA runtime/base image the CLI currently uses
when --cuda is omitted (e.g., "This command uses
nvcr.io/nvidia/cuda:12.6.0-runtime-ubuntu22.04 as the CUDA base image by
default"); edit the cell containing the command to include the chosen fix and
reference the monai-deploy package invocation so reviewers can find it.
In `@tests/unit/test_decoder_nvimgcodec.py`:
- Around line 40-60: The teardown of the _configure_decoder_test_console_logging
fixture must restore each logger's prior configuration instead of forcing
logging.NOTSET; capture each logger's original level (and propagate if you want
to restore that too) when you add the handler (e.g., record tuples of (logger,
original_level, original_propagate) for loggers obtained via
logging.getLogger(_DECODER_LOGGER) and logging.getLogger(__name__)), then in the
teardown loop restore logger.setLevel(original_level), restore logger.propagate
to original_propagate if recorded, remove the handler, and close it to avoid
leaking config between tests.
- Around line 35-36: The test sets _LOG_DATE_FORMAT = "%H:%M:%S.%f" which stdlib
logging.strftime does not support, so replace it with datefmt="%H:%M:%S" and
include milliseconds in the format string by updating _LOG_FORMAT to inject
".%(msecs)03d" into the timestamp (i.e. modify _LOG_FORMAT and _LOG_DATE_FORMAT
used with logging.Formatter), or alternatively implement/use a custom Formatter
that formats microseconds; update the logging.Formatter(...) call to use the new
_LOG_FORMAT and _LOG_DATE_FORMAT accordingly.
---
Nitpick comments:
In `@README.md`:
- Line 41: The README's compatibility sentence is ambiguous about whether "up to
version 4.2" includes all 4.2.x releases while requirements.txt uses
">=4.0.0,<4.3.0"; update the README to explicitly state the intended range
(e.g., "compatible with holoscan-cli versions 4.0 through 4.2 (including
4.2.x)") so it matches the requirements.txt constraint, referencing the package
names holoscan-cu13 and holoscan-cli and the version constraint expression
>=4.0.0,<4.3.0 to ensure clarity.
In `@setup.cfg`:
- Line 27: The setup.cfg entry for the package "holoscan-cu13" lacks version
constraints; update the package specification in setup.cfg (the install_requires
or options.install_requires list where "holoscan-cu13" appears) to match
requirements.txt by using "holoscan-cu13>=4.0.0,<4.3.0" so installations via
setup.cfg/packaging use the same bounds as pip installs.
🪄 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: 24c5a3f8-9f85-4530-8ab4-96f5bd21b514
📒 Files selected for processing (17)
.github/workflows/pr.ymlREADME.mdTHIRD_PARTY_NOTICES/nvidia-nvimgcodec_Apache2.0_LICENSE.txtmonai/deploy/operators/decoder_nvimgcodec.pymonai/deploy/operators/dicom_series_to_volume_operator.pynotebooks/tutorials/01_simple_app.ipynbnotebooks/tutorials/02_mednist_app-prebuilt.ipynbnotebooks/tutorials/02_mednist_app.ipynbnotebooks/tutorials/03_segmentation_app.ipynbnotebooks/tutorials/04_monai_bundle_app.ipynbnotebooks/tutorials/05_multi_model_app.ipynbrequirements-dev.txtrequirements-examples.txtrequirements.txtrunsetup.cfgtests/unit/test_decoder_nvimgcodec.py
Signed-off-by: M Q <mingmelvinq@nvidia.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
run (1)
342-342:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPin
nvidia-cuda-runtimeto a specific 13.x release for ReadTheDocs reproducibilityFile: run
Lines: 342-342run_command ${MONAI_PY_EXE} -m pip install "nvidia-cuda-runtime==13.*"That
==13.*constraint lets RTD builds float to newernvidia-cuda-runtime13.x versions over time (latest13.*on PyPI is13.3.29); pin to the exact version you want (e.g.,nvidia-cuda-runtime==13.3.29) to keep doc builds consistent.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@run` at line 342, The pip install call using run_command with ${MONAI_PY_EXE} currently pins nvidia-cuda-runtime to a floating 13.x range ("nvidia-cuda-runtime==13.*"); change that invocation to pin to a specific 13.x release (e.g., replace the "nvidia-cuda-runtime==13.*" argument passed to run_command/${MONAI_PY_EXE} with the exact version you want, such as "nvidia-cuda-runtime==13.3.29") so ReadTheDocs builds are reproducible.
🧹 Nitpick comments (1)
.github/workflows/pr.yml (1)
35-35: ⚡ Quick winRemove redundant CUDA runtime installations.
The
nvidia-cuda-runtime==13.*package is installed four times across different steps (lines 26, 35, 44, 49), but all steps activate the same.venvvirtual environment. Since line 26 already installs the package and lines 29-30 persist the library paths toGITHUB_ENV, the reinstalls in later steps are unnecessary and slow down CI.♻️ Proposed fix to remove redundant installs
Remove the redundant installations at lines 35, 44, and 49:
- name: Check formatting run: | source .venv/bin/activate - python3 -m pip install "nvidia-cuda-runtime==13.*" python3 -c 'import sys; print(sys.executable)'- name: Run Unit tests run: | source .venv/bin/activate - python3 -m pip install "nvidia-cuda-runtime==13.*" ./run test all unit- name: Coverage run: | source .venv/bin/activate - python3 -m pip install "nvidia-cuda-runtime==13.*" coverage xmlAlso applies to: 44-44, 49-49
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/pr.yml at line 35, Remove the redundant "python3 -m pip install \"nvidia-cuda-runtime==13.*\"" invocations that reappear after the initial installation (the first install that activates .venv and persists CUDA lib paths to GITHUB_ENV); keep only the first install step and delete the duplicate commands (the later occurrences that run while the same .venv is active) so subsequent steps rely on the already-installed runtime.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@run`:
- Line 342: The pip install call using run_command with ${MONAI_PY_EXE}
currently pins nvidia-cuda-runtime to a floating 13.x range
("nvidia-cuda-runtime==13.*"); change that invocation to pin to a specific 13.x
release (e.g., replace the "nvidia-cuda-runtime==13.*" argument passed to
run_command/${MONAI_PY_EXE} with the exact version you want, such as
"nvidia-cuda-runtime==13.3.29") so ReadTheDocs builds are reproducible.
---
Nitpick comments:
In @.github/workflows/pr.yml:
- Line 35: Remove the redundant "python3 -m pip install
\"nvidia-cuda-runtime==13.*\"" invocations that reappear after the initial
installation (the first install that activates .venv and persists CUDA lib paths
to GITHUB_ENV); keep only the first install step and delete the duplicate
commands (the later occurrences that run while the same .venv is active) so
subsequent steps rely on the already-installed runtime.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 53c1761c-69cb-49e1-b633-bd638b970eff
📒 Files selected for processing (3)
.github/workflows/pr.ymlREADME.mdrun
✅ Files skipped from review due to trivial changes (1)
- README.md
Signed-off-by: M Q <mingmelvinq@nvidia.com>
Signed-off-by: M Q <mingmelvinq@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Line 42: Update the compound adjective in the README: find the sentence
containing "Key runtime dependencies also include [nvidia-nvimgcodec]... GPU
accelerated DICOM image decoding" and change "GPU accelerated" to the hyphenated
form "GPU-accelerated" so it reads "GPU-accelerated DICOM image decoding".
- Line 41: Update the README sentence that currently reads "Ubuntu 22.04 with
glibc 2.39 or above" to correctly state that Ubuntu 22.04 requires glibc 2.35+
for the holoscan-cu13 wheels (or add a clarifying parenthetical that glibc 2.39
applies to other platform builds such as Ubuntu 24.04/aarch64); modify the line
referencing holoscan-cu13/holoscan-cli compatibility so it mentions "glibc
2.35+" for Ubuntu 22.04 while preserving the note that other platforms may need
2.39.
🪄 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: 147e8aa0-6f5d-4354-b4b6-99a1701f03ee
📒 Files selected for processing (3)
README.mdmonai/deploy/operators/decoder_nvimgcodec.pytests/unit/test_decoder_nvimgcodec.py
🚧 Files skipped from review as they are similar to previous changes (2)
- monai/deploy/operators/decoder_nvimgcodec.py
- tests/unit/test_decoder_nvimgcodec.py
Signed-off-by: M Q <mingmelvinq@nvidia.com>
|
|
This change will bring in cuda-toolkit v13.0.2, which should help avoid the runtime issue that #582 trys to prevent. According to nvJPEG developers, the internal nvJPEG "fix is restricted to 13.0.2+ nvjpeg packages only" |
Summary by CodeRabbit
Chores
Documentation
Tests