fix: update Dockerfile and requirements-dev.txt for MONAI 1.6 tutorial compatibility#8912
Open
garciadias wants to merge 3 commits into
Open
fix: update Dockerfile and requirements-dev.txt for MONAI 1.6 tutorial compatibility#8912garciadias wants to merge 3 commits into
garciadias wants to merge 3 commits into
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughRewrites the image pip constraint file to keep the existing numpy pin and add setuptools<71, updates requirements-dev.txt with setuptools<71, tighter transformers and mlflow ranges, and new dev deps (aim on Linux, lightning>=2.0). Extends the Docker pip install step to install requirements-dev plus papermill, jupytext, autopep8, autoflake, and ipywidgets. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@Dockerfile`:
- Around line 44-46: The Dockerfile currently re-specifies mlflow and
transformers in the extra pip install line, creating a conflicting second source
of truth; remove the "mlflow<3.0" and "transformers<5.0" entries from the python
-m pip install line that also installs
papermill/jupytext/autopep8/autoflake/ipywidgets so that mlflow and transformers
are only resolved from requirements-dev.txt (which pins >=2.12.2,<3.0 and
>=4.41.0,<5.0 respectively); keep the separate install for the other tools
unchanged.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1a02d9ae-23fb-489b-b718-130352617025
📒 Files selected for processing (2)
Dockerfilerequirements-dev.txt
…l compatibility Signed-off-by: R. Garcia-Dias <rafaelagd@gmail.com> Base image updated to nvcr.io/nvidia/pytorch:25.12-py3 (aligned with upstream dev). Pin mlflow<3.0 (3.x broken on Python 3.12) and transformers<5.0 (5.x references torch.float8_e8m0fnu absent in older PyTorch builds). Add aim and lightning>=2.0 as declared dependencies. Remove Python 3.10 version caps from cucim and onnxruntime. Rebuild pip constraint file to keep numpy==1.26.4 and add setuptools<71. Add papermill/jupytext/autopep8/autoflake/ipywidgets for tutorial runner.
garciadias
force-pushed
the
fix/monai_1_6_release_notebooks
branch
from
9a5bfeb to
a6c8f3e
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
Dockerfile (1)
30-30: 💤 Low valueUpdate version reference in comment.
If the base image is corrected to
25.03-py3(per line 14 review comment), update this comment to reference "PyTorch nv25.03" instead of "nv25.12" for consistency.🤖 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 `@Dockerfile` at line 30, Update the inline comment that mentions the PyTorch build to match the base image change: replace the "nv25.12" reference with "nv25.03" in the Dockerfile comment that currently reads about keeping the numpy==1.26.4 pin so it consistently references "PyTorch nv25.03".
🤖 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 `@Dockerfile`:
- Line 32: The comment "remove jupytext/isort pins" is misleading because the
script currently overwrites /etc/pip/constraint.txt with only numpy and
setuptools constraints (removing everything implicitly) — either update the
comment to accurately state that the constraints file is being replaced, or
change the implementation to perform a selective removal of only jupytext and
isort pins (e.g., edit /etc/pip/constraint.txt in-place by filtering out lines
matching jupytext and isort) so the behavior matches the comment; reference the
Dockerfile comment "remove jupytext/isort pins" and the code path that writes
/etc/pip/constraint.txt.
- Line 14: Update the PYTORCH_IMAGE ARG value from
nvcr.io/nvidia/pytorch:25.12-py3 to nvcr.io/nvidia/pytorch:25.03-py3 to match
the PR target; adjust the constraint-file handling so the grep/sed pipeline that
looks for '^numpy==' in /etc/pip/constraint.txt does not cause a failing build
when no match exists (make the grep tolerant or fallback instead of aborting);
and change the RUN step that currently rewrites /etc/pip/constraint.txt to only
the numpy pin to instead selectively remove pins for jupytext and isort while
preserving other constraints (including keeping setuptools<71 if present).
Ensure you update the commands referencing "grep '^numpy=='", the ARG
PYTORCH_IMAGE, and the RUN that claims to "remove jupytext/isort pins" (so
behavior matches the comment).
- Line 33: The RUN step that extracts numpy constraints using grep can fail the
Docker build when there are no matches; modify the command that writes
/tmp/new_constraints.txt from /etc/pip/constraint.txt (the current grep
'^numpy==' invocation) to tolerate no matches (e.g., append "|| true" or use a
grep option that returns 0) so the step never exits non‑zero, then ensure the
subsequent operations still overwrite the original constraint file and append
"setuptools<71" as intended; update the surrounding comment to accurately state
that this step overwrites the constraint file with any numpy== lines (if
present) and then appends setuptools<71.
---
Nitpick comments:
In `@Dockerfile`:
- Line 30: Update the inline comment that mentions the PyTorch build to match
the base image change: replace the "nv25.12" reference with "nv25.03" in the
Dockerfile comment that currently reads about keeping the numpy==1.26.4 pin so
it consistently references "PyTorch nv25.03".
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ea9ecbc0-57b9-4b2e-bc72-0fbecc67a32a
📒 Files selected for processing (2)
Dockerfilerequirements-dev.txt
💤 Files with no reviewable changes (1)
- requirements-dev.txt
Contributor
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 3
🧹 Nitpick comments (1)
Dockerfile (1)
30-30: 💤 Low valueUpdate version reference in comment.
If the base image is corrected to
25.03-py3(per line 14 review comment), update this comment to reference "PyTorch nv25.03" instead of "nv25.12" for consistency.🤖 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 `@Dockerfile` at line 30, Update the inline comment that mentions the PyTorch build to match the base image change: replace the "nv25.12" reference with "nv25.03" in the Dockerfile comment that currently reads about keeping the numpy==1.26.4 pin so it consistently references "PyTorch nv25.03".
🤖 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 `@Dockerfile`:
- Line 32: The comment "remove jupytext/isort pins" is misleading because the
script currently overwrites /etc/pip/constraint.txt with only numpy and
setuptools constraints (removing everything implicitly) — either update the
comment to accurately state that the constraints file is being replaced, or
change the implementation to perform a selective removal of only jupytext and
isort pins (e.g., edit /etc/pip/constraint.txt in-place by filtering out lines
matching jupytext and isort) so the behavior matches the comment; reference the
Dockerfile comment "remove jupytext/isort pins" and the code path that writes
/etc/pip/constraint.txt.
- Line 14: Update the PYTORCH_IMAGE ARG value from
nvcr.io/nvidia/pytorch:25.12-py3 to nvcr.io/nvidia/pytorch:25.03-py3 to match
the PR target; adjust the constraint-file handling so the grep/sed pipeline that
looks for '^numpy==' in /etc/pip/constraint.txt does not cause a failing build
when no match exists (make the grep tolerant or fallback instead of aborting);
and change the RUN step that currently rewrites /etc/pip/constraint.txt to only
the numpy pin to instead selectively remove pins for jupytext and isort while
preserving other constraints (including keeping setuptools<71 if present).
Ensure you update the commands referencing "grep '^numpy=='", the ARG
PYTORCH_IMAGE, and the RUN that claims to "remove jupytext/isort pins" (so
behavior matches the comment).
- Line 33: The RUN step that extracts numpy constraints using grep can fail the
Docker build when there are no matches; modify the command that writes
/tmp/new_constraints.txt from /etc/pip/constraint.txt (the current grep
'^numpy==' invocation) to tolerate no matches (e.g., append "|| true" or use a
grep option that returns 0) so the step never exits non‑zero, then ensure the
subsequent operations still overwrite the original constraint file and append
"setuptools<71" as intended; update the surrounding comment to accurately state
that this step overwrites the constraint file with any numpy== lines (if
present) and then appends setuptools<71.
---
Nitpick comments:
In `@Dockerfile`:
- Line 30: Update the inline comment that mentions the PyTorch build to match
the base image change: replace the "nv25.12" reference with "nv25.03" in the
Dockerfile comment that currently reads about keeping the numpy==1.26.4 pin so
it consistently references "PyTorch nv25.03".
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ea9ecbc0-57b9-4b2e-bc72-0fbecc67a32a
📒 Files selected for processing (2)
Dockerfilerequirements-dev.txt
💤 Files with no reviewable changes (1)
- requirements-dev.txt
🛑 Comments failed to post (3)
Dockerfile (3)
14-14:
⚠️ Potential issue | 🔴 Critical🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Description: Check NVIDIA NGC catalog for PyTorch 25.03 and 25.12 availability echo "Checking NVIDIA PyTorch container tags for 25.03 and 25.12..." # Query NGC API for available PyTorch tags curl -s 'https://catalog.ngc.nvidia.com/api/resources/versions?orgName=nvidia&teamName=pytorch&name=pytorch&pageSize=100' \ | jq -r '.results[].name' \ | grep -E '^25\.(03|12)-py3$' || echo "No matching tags found"Repository: Project-MONAI/MONAI
Length of output: 299
🏁 Script executed:
#!/bin/bash set -euo pipefail # Show Dockerfile with line numbers around the claimed sections echo "=== Dockerfile (with line numbers, full file if <=200 lines) ===" wc -l Dockerfile if [ "$(wc -l Dockerfile | awk '{print $1}')" -le 200 ]; then cat -n Dockerfile else nl -ba Dockerfile | sed -n '1,120p' echo "..." nl -ba Dockerfile | sed -n '120,240p' fi # Find any repo references to the two PyTorch image tags echo echo "=== References to 25.03-py3 ===" rg -n --hidden --glob '!.git/*' '25\.03-py3' . || true echo echo "=== References to 25.12-py3 ===" rg -n --hidden --glob '!.git/*' '25\.12-py3' . || true # Also search for the constraint-rewrite comment / claims (best-effort) echo echo "=== References to constraint.txt modifications ===" rg -n --hidden --glob '!.git/*' 'constraint\.txt' Dockerfile . || true echo echo "=== References to jupytext/isort pins mention ===" rg -n --hidden --glob '!.git/*' 'jupytext|isort' Dockerfile . || trueRepository: Project-MONAI/MONAI
Length of output: 6956
Align PYTORCH_IMAGE tag with PR target; harden constraint rewrite.
- Dockerfile line 14 sets
ARG PYTORCH_IMAGE=nvcr.io/nvidia/pytorch:25.12-py3, but the PR objectives/scope statenvcr.io/nvidia/pytorch:25.03-py3(version mismatch).- Dockerfile lines 33-35:
grep '^numpy==' /etc/pip/constraint.txt ... && ... cp ...will fail the build if the constraint file doesn’t contain a^numpy==...line.- Dockerfile line 32 claims to “remove jupytext/isort pins”, but the RUN step actually rewrites the constraint file to only keep the extracted NumPy pin (plus
setuptools<71), not selectively removing just jupytext/isort.🔧 Proposed fix
-ARG PYTORCH_IMAGE=nvcr.io/nvidia/pytorch:25.12-py3 +ARG PYTORCH_IMAGE=nvcr.io/nvidia/pytorch:25.03-py3🤖 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 `@Dockerfile` at line 14, Update the PYTORCH_IMAGE ARG value from nvcr.io/nvidia/pytorch:25.12-py3 to nvcr.io/nvidia/pytorch:25.03-py3 to match the PR target; adjust the constraint-file handling so the grep/sed pipeline that looks for '^numpy==' in /etc/pip/constraint.txt does not cause a failing build when no match exists (make the grep tolerant or fallback instead of aborting); and change the RUN step that currently rewrites /etc/pip/constraint.txt to only the numpy pin to instead selectively remove pins for jupytext and isort while preserving other constraints (including keeping setuptools<71 if present). Ensure you update the commands referencing "grep '^numpy=='", the ARG PYTORCH_IMAGE, and the RUN that claims to "remove jupytext/isort pins" (so behavior matches the comment).
32-32:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winComment claims to "remove jupytext/isort pins" but code replaces entire file.
Line 32 states the script "remove[s] jupytext/isort pins", but lines 33-35 actually overwrite
/etc/pip/constraint.txtwith only the numpy and setuptools constraints. The removal is implicit (by not copying other constraints), not selective.📝 Proposed clarification
-# - remove jupytext/isort pins so the tutorial runner can install its required versions +# - discard all other constraints (including jupytext/isort) so the tutorial runner can install required versions🤖 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 `@Dockerfile` at line 32, The comment "remove jupytext/isort pins" is misleading because the script currently overwrites /etc/pip/constraint.txt with only numpy and setuptools constraints (removing everything implicitly) — either update the comment to accurately state that the constraints file is being replaced, or change the implementation to perform a selective removal of only jupytext and isort pins (e.g., edit /etc/pip/constraint.txt in-place by filtering out lines matching jupytext and isort) so the behavior matches the comment; reference the Dockerfile comment "remove jupytext/isort pins" and the code path that writes /etc/pip/constraint.txt.
33-33:
⚠️ Potential issue | 🔴 Critical🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Description: Check if NVIDIA PyTorch 25.12-py3 base image has numpy constraint echo "Attempting to pull base image and inspect /etc/pip/constraint.txt..." docker pull nvcr.io/nvidia/pytorch:25.12-py3 2>&1 | head -20 if docker image inspect nvcr.io/nvidia/pytorch:25.12-py3 >/dev/null 2>&1; then echo -e "\n=== Contents of /etc/pip/constraint.txt ===" docker run --rm nvcr.io/nvidia/pytorch:25.12-py3 cat /etc/pip/constraint.txt 2>/dev/null || echo "File not found" echo -e "\n=== Lines starting with numpy== ===" docker run --rm nvcr.io/nvidia/pytorch:25.12-py3 grep '^numpy==' /etc/pip/constraint.txt 2>/dev/null || echo "No numpy constraint found" else echo "Image not available locally or remotely" fiRepository: Project-MONAI/MONAI
Length of output: 217
Make the numpy-constraint rewrite resilient to missing matches
grep '^numpy==' /etc/pip/constraint.txtexits non-zero when there’s nonumpy==...line, so the chainedRUNwill fail the Docker build. Also align the surrounding comment with the actual behavior: the step overwrites the constraint file (keeping only numpy matches) and then appendssetuptools<71.🤖 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 `@Dockerfile` at line 33, The RUN step that extracts numpy constraints using grep can fail the Docker build when there are no matches; modify the command that writes /tmp/new_constraints.txt from /etc/pip/constraint.txt (the current grep '^numpy==' invocation) to tolerate no matches (e.g., append "|| true" or use a grep option that returns 0) so the step never exits non‑zero, then ensure the subsequent operations still overwrite the original constraint file and append "setuptools<71" as intended; update the surrounding comment to accurately state that this step overwrites the constraint file with any numpy== lines (if present) and then appends setuptools<71.
…mpatibility MetricsReloaded's setup.py imports pkg_resources, which was removed from setuptools>=71. The Dockerfile already constrains this via /etc/pip/constraint.txt, but CI runners (GitHub Actions) have no such constraint and fail with ModuleNotFoundError: No module named 'pkg_resources' during MetricsReloaded build. Signed-off-by: R. Garcia-Dias <rafaelagd@gmail.com>
aim's build requires aimrocks (C extension with no Windows wheels), causing metadata-generation-failed on Windows CI runners. aim is only used by the spleen_segmentation_aim tutorial which requires a Linux/GPU environment anyway. Signed-off-by: R. Garcia-Dias <rafaelagd@gmail.com>
5 tasks
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
nvcr.io/nvidia/pytorch:24.10-py3to25.03-py3to support RTX 5090 (Blackwell, SM_120/CUDA 12.8); remove the now-obsoletetorch.patchONNX revert which was specific to 24.10mlflow<3.0— mlflow 3.x is broken on Python 3.12 due to a relative import inmlflow.utils.uv_utils(from .. import zippfails at top-level scope); this caused 4 tutorial notebooks to fail in our CI runtransformers<5.0— transformers 5.x referencestorch.float8_e8m0fnuwhich does not exist in the nv25.03 build of PyTorch 2.7; this caused the HuggingFace tutorial to failaimandlightning>=2.0as declared dependencies inrequirements-dev.txt(were previously undeclared but required by tutorial notebooks)numpy==1.26.4(nv25.03 PyTorch compiled against NumPy 1.x) and addsetuptools<71(newer setuptools droppedpkg_resourcesneeded by legacysetup.pyin git-sourced packages like MetricsReloaded and segment-anything)python_version <= '3.10'caps fromcucim,onnxruntime, andtransformers— these restrictions were keeping packages out of the Python 3.12 image unnecessarilypapermill,jupytext,autopep8,autoflake, andipywidgetsdirectly in the Dockerfile so the tutorial runner is self-containedContext
These changes were identified by running the full MONAI tutorial test suite in a fresh Docker build against a MONAI 1.6 dev branch and comparing results with a native conda reference run (Eric's run,
eccefc57). The rerun with stderr captured (runner_output_our_only.logs) confirmed the specific error for each notebook group.Type of change
Test plan
docker build -t monai_1_6:latest .bash run_our_only.sh 2>&1 | tee runner_output_v2.logsinside the containerhugging_face/hugging_face_pipeline_for_monai.ipynbpasses (R3)experiment_management/spleen_segmentation_aim.ipynbpasses (R6)🤖 Generated with Claude Code