Skip to content

Update download link - #7

Merged
heyufan1995 merged 2 commits into
NVIDIA-Medtech:mainfrom
heyufan1995:main
Mar 10, 2026
Merged

Update download link#7
heyufan1995 merged 2 commits into
NVIDIA-Medtech:mainfrom
heyufan1995:main

Conversation

@heyufan1995

Copy link
Copy Markdown
Contributor

No description provided.

heyufan1995 and others added 2 commits March 10, 2026 17:35
Signed-off-by: heyufan <heyufan1995@gmail.com>
@heyufan1995
heyufan1995 merged commit 6388d8f into NVIDIA-Medtech:main Mar 10, 2026
1 check failed
@greptile-apps

greptile-apps Bot commented Mar 10, 2026

Copy link
Copy Markdown

Greptile Summary

This PR updates the model download method from wget to the huggingface_hub CLI (hf download) for both NV-Segment-CT and NV-Segment-CTMR, adds the huggingface_hub package (and several brain-preprocessing dependencies) to requirements.txt, bumps torch to 2.1.2 with a matching torchvision in the CTMR package, fixes a bug in run_brain_segmentation.sh where --output_dir was not passed to monai.bundle run, and adds a new ctmr.png visual asset to the root README.

Key changes:

  • Download instructions: Both READMEs now use hf download ... --local-dir + mv + rmdir instead of wget, requiring the huggingface_hub package (added to both requirements.txt files).
  • Shell script bug fix: --output_dir "$output_dir" is now forwarded to monai.bundle run in run_brain_segmentation.sh, ensuring segmentation output is written to the user-specified directory (consistent with how $preprocess_tmp_seg is resolved in Step 4).
  • Dependency updates (CTMR): torch upgraded from 2.0.1 → 2.1.2, torchvision==0.16.2 added, and several brain-preprocessing libraries (pymedio, SimpleITK, pydicom, statsmodels, antspyx) added.
  • Documentation: Root README.md table now links class counts to label_dict.json files and embeds a new CTMR architecture image.

Confidence Score: 4/5

  • Safe to merge with minor robustness improvements recommended in the download cleanup commands.
  • The changes are straightforward: a download-method migration, a clear bug fix in the shell script, dependency additions, and documentation improvements. The only functional concern is that rmdir used after hf download --local-dir could fail if huggingface_hub writes any extra files (metadata, .gitattributes) into the subdirectory — rm -rf would be safer. The "Option 1:" label in the CTMR README without a matching Option 2 is a minor documentation confusion. Everything else (torch upgrade, new deps, bug fix) looks correct.
  • NV-Segment-CT/docs/README.md and NV-Segment-CTMR/docs/README.md — both use rmdir which could fail depending on the huggingface_hub version's download behavior.

Important Files Changed

Filename Overview
README.md Documentation-only change: adds a hyperlink and "(7 types of tumors)" label to the comparison table, and embeds the new ctmr.png image. No issues found.
NV-Segment-CT/docs/README.md Replaces wget download with hf download + mv + rmdir pipeline. The rmdir call could fail on versions of huggingface_hub that write extra files (metadata, .gitattributes) into the subdirectory; rm -rf is safer.
NV-Segment-CT/requirements.txt Adds huggingface_hub (unpinned) to support the new hf download CLI command. Missing newline at end of file.
NV-Segment-CTMR/brain_t1_preprocess/run_brain_segmentation.sh Bug fix: adds --output_dir "$output_dir" to the monai.bundle run call so segmentation output is written to the user-specified directory, making it consistent with how $preprocess_tmp_seg is resolved in Step 4.
NV-Segment-CTMR/docs/README.md Replaces wget with hf download pipeline. Same rmdir fragility concern as the CT README; additionally the "Option 1:" label has no corresponding Option 2, which is confusing.
NV-Segment-CTMR/docs/ctmr.png New binary image asset added; referenced correctly from the root README.
NV-Segment-CTMR/requirements.txt Updates torch from 2.0.1 to 2.1.2 and adds compatible torchvision==0.16.2; adds brain-preprocessing dependencies (pymedio, SimpleITK, pydicom, statsmodels, antspyx) and huggingface_hub. Missing newline at end of file.

Sequence Diagram

sequenceDiagram
    participant User
    participant Shell as run_brain_segmentation.sh
    participant SynthStrip
    participant Preprocess as preprocess.py
    participant MONAI as monai.bundle run
    participant Revert as revert_preprocess.py

    User->>Shell: --input brain_t1.nii.gz --output_dir results/
    Shell->>SynthStrip: skull strip input → temp/skull_stripped.nii.gz
    SynthStrip-->>Shell: skull_stripped.nii.gz
    Shell->>Preprocess: align to LUMIR template → temp/preprocessed.nii.gz
    Preprocess-->>Shell: preprocessed.nii.gz + meta.json
    Shell->>MONAI: run inference --output_dir results/ (NEW)
    Note over MONAI: Previously output_dir was not passed,<br/>causing segmentation to write to wrong location
    MONAI-->>Shell: results/preprocessed/preprocessed_trans.nii.gz
    Shell->>Revert: revert to original space → results/basename_trans.nii.gz
    Revert-->>Shell: final_output
    Shell-->>User: Segmentation complete
Loading

Last reviewed commit: 7dc37ae

wget -O NV-Segment-CT/models/model.pt https://huggingface.co/nvidia/NV-Segment-CT/resolve/main/vista3d_pretrained_model/model.pt
hf download nvidia/NV-Segment-CT vista3d_pretrained_model/model.pt --local-dir NV-Segment-CT/models/ && \
mv NV-Segment-CT/models/vista3d_pretrained_model/model.pt NV-Segment-CT/models/model.pt && \
rmdir NV-Segment-CT/models/vista3d_pretrained_model

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rmdir may fail if extra files are present

rmdir only succeeds on empty directories. Depending on the version of huggingface_hub, the --local-dir download may also create additional metadata files (e.g., .gitattributes, lock files) inside vista3d_pretrained_model/, causing rmdir to fail silently (or halt the script with set -e). Using rm -rf would be more robust:

Suggested change
rmdir NV-Segment-CT/models/vista3d_pretrained_model
rm -rf NV-Segment-CT/models/vista3d_pretrained_model

Comment on lines +28 to +31
# Option 1: Download using hf and move to expected location
hf download nvidia/NV-Segment-CTMR vista3d_pretrained_model/model.pt --local-dir NV-Segment-CTMR/models/ && \
mv NV-Segment-CTMR/models/vista3d_pretrained_model/model.pt NV-Segment-CTMR/models/model.pt && \
rmdir NV-Segment-CTMR/models/vista3d_pretrained_model

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Option 1" label without a corresponding "Option 2"

The comment labels this step as Option 1: Download using hf and move to expected location, but there is no Option 2 in the instructions. This could leave users confused about whether there is an alternative method. If a second option (e.g., wget or huggingface-cli) was intentionally removed, the Option 1: label should be dropped:

Suggested change
# Option 1: Download using hf and move to expected location
hf download nvidia/NV-Segment-CTMR vista3d_pretrained_model/model.pt --local-dir NV-Segment-CTMR/models/ && \
mv NV-Segment-CTMR/models/vista3d_pretrained_model/model.pt NV-Segment-CTMR/models/model.pt && \
rmdir NV-Segment-CTMR/models/vista3d_pretrained_model
# Download using hf and move to expected location
hf download nvidia/NV-Segment-CTMR vista3d_pretrained_model/model.pt --local-dir NV-Segment-CTMR/models/ && \
mv NV-Segment-CTMR/models/vista3d_pretrained_model/model.pt NV-Segment-CTMR/models/model.pt && \
rmdir NV-Segment-CTMR/models/vista3d_pretrained_model

# Option 1: Download using hf and move to expected location
hf download nvidia/NV-Segment-CTMR vista3d_pretrained_model/model.pt --local-dir NV-Segment-CTMR/models/ && \
mv NV-Segment-CTMR/models/vista3d_pretrained_model/model.pt NV-Segment-CTMR/models/model.pt && \
rmdir NV-Segment-CTMR/models/vista3d_pretrained_model

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rmdir may fail if extra files are present

Same concern as in NV-Segment-CT/docs/README.md: rmdir is not idempotent and will fail if huggingface_hub's download writes any extra files (metadata, lock files, .gitattributes) into the subdirectory. Prefer rm -rf for reliability:

Suggested change
rmdir NV-Segment-CTMR/models/vista3d_pretrained_model
rm -rf NV-Segment-CTMR/models/vista3d_pretrained_model

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant