Skip to content

[codex] Trigger Hugging Face access during inference - #13

Merged
heyufan1995 merged 2 commits into
NVIDIA-Medtech:mainfrom
heyufan1995:codex/hf-download-per-inference
May 18, 2026
Merged

[codex] Trigger Hugging Face access during inference#13
heyufan1995 merged 2 commits into
NVIDIA-Medtech:mainfrom
heyufan1995:codex/hf-download-per-inference

Conversation

@heyufan1995

Copy link
Copy Markdown
Contributor

Summary

  • Move model-weight preparation into inference for both NV-Segment-CT and NV-Segment-CTMR.
  • Touch the Hugging Face model repo config.json on every inference run so cached local weights still register a Hub access.
  • Automatically prepare models/model.pt from the Hugging Face cache if the local checkpoint path is missing.
  • Update README setup docs to remove the required manual hf download step.
  • Add batch inference smoke-test scripts for both model folders.
  • Fix batch output path handling by using real paths for input/output roots, which avoids symlink alias issues on DFW.

Why

Previously setup downloaded model weights once, so later inference runs used only models/model.pt and did not touch Hugging Face. This follows the Cosmos-style runtime checkpoint access pattern while preserving the local MONAI checkpoint path expected by existing configs.

Validation

Local:

  • python3 -m py_compile NV-Segment-CT/scripts/huggingface_download.py NV-Segment-CTMR/scripts/huggingface_download.py
  • python3 -m json.tool for changed inference, mgpu, and batch configs
  • bash -n for both scripts/test_batch_inference.sh scripts

DFW:

  • CTMR single-image inference passed and logged Hugging Face HEAD/GET for nvidia/NV-Segment-CTMR/config.json.
  • CT single-image inference passed and prepared missing NV-Segment-CT/models/model.pt from Hugging Face.
  • CTMR batch smoke test passed, including resume skip.
  • CT batch smoke test passed, including resume skip.

DFW output paths:

  • /lustre/fsw/portfolios/healthcareeng/users/yufanh/experiments/nvseg_ctmr_batch_test/output/s0289/s0289_trans.nii.gz
  • /lustre/fsw/portfolios/healthcareeng/users/yufanh/experiments/nvseg_ct_batch_test/output/spleen_03/spleen_03_trans.nii.gz

@heyufan1995
heyufan1995 marked this pull request as ready for review May 18, 2026 20:05
@heyufan1995
heyufan1995 merged commit df30612 into NVIDIA-Medtech:main May 18, 2026
1 check passed
@heyufan1995
heyufan1995 deleted the codex/hf-download-per-inference branch May 18, 2026 20:08
@greptile-apps

greptile-apps Bot commented May 18, 2026

Copy link
Copy Markdown

Greptile Summary

This PR moves Hugging Face model-weight preparation into the inference path for both NV-Segment-CT and NV-Segment-CTMR, eliminating the manual hf download setup step and registering a Hub access on every inference run by touching config.json via force_download=True.

  • huggingface_download.py (new, identical in both bundles) adds prepare_huggingface_checkpoint and touch_huggingface_download_counter; the former is called as the first initialize step in both inference.json and mgpu_inference.json, with a dist.barrier() correctly placed between the download and CheckpointLoader in the multi-GPU config.
  • batch_inference.json (both bundles) switches from os.path.abspath to os.path.realpath for input/output roots and explicitly passes output_dir_abs to the postprocessing transform, fixing output-path mismatches caused by symlink aliases on DFW.
  • New scripts/test_batch_inference.sh smoke tests in both bundles validate first-run processing, HF counter-touch log output, and resume-skip behavior.

Confidence Score: 4/5

The single-GPU and multi-GPU initialization flows are structurally correct; the one path worth hardening before wide production use is the broken-symlink recovery in huggingface_download.py.

The core change — calling prepare_huggingface_checkpoint at inference time and placing dist.barrier() between the download and CheckpointLoader — is implemented correctly. The _is_rank_zero env-var approach covers all standard single-node and multi-node torchrun cases. The realpath fix for batch output paths is a clean, targeted repair. The one real concern is in prepare_huggingface_checkpoint: when a symlink at local_path is broken (HF cache cleared), symlink_to raises FileExistsError, the OSError handler falls through to shutil.copy2, and on Linux that call writes through the broken symlink to its target — which may itself be unreachable, leaving inference unable to load the checkpoint until the stale symlink is manually removed. This is the same code in both bundle scripts.

Both NV-Segment-CT/scripts/huggingface_download.py and NV-Segment-CTMR/scripts/huggingface_download.py — specifically the prepare_huggingface_checkpoint function around the symlink-creation and copy fallback logic.

Important Files Changed

Filename Overview
NV-Segment-CT/scripts/huggingface_download.py New file that touches HF download counter and prepares the local checkpoint at inference time; has a broken-symlink edge case in the OSError fallback and a rank-detection fragility in multi-node SLURM+torchrun environments.
NV-Segment-CTMR/scripts/huggingface_download.py Identical to the CT variant; shares both the broken-symlink and rank-detection issues.
NV-Segment-CT/configs/mgpu_inference.json Adds HF checkpoint preparation before dist.barrier(), correctly ensuring only rank 0 downloads while all ranks synchronize before CheckpointLoader runs.
NV-Segment-CTMR/configs/mgpu_inference.json Same change as the CT variant; barrier placement is correct.
NV-Segment-CT/configs/batch_inference.json Switches to os.path.realpath for both input/output roots and plumbs output_dir_abs into the postprocessing transform, fixing symlink-alias path mismatches on DFW.
NV-Segment-CTMR/configs/batch_inference.json Identical realpath and output_dir_abs fix as the CT batch config.
NV-Segment-CT/scripts/test_batch_inference.sh New smoke-test script; validates first-run processing, HF counter touch log line, and resume-skip behavior.
NV-Segment-CTMR/scripts/test_batch_inference.sh New smoke-test script for CTMR; same structure as the CT test with MODALITY override.
NV-Segment-CT/configs/inference.json Adds HF config keys and checkpoint_path, calls prepare_huggingface_checkpoint as first initialize step; straightforward and correct for single-GPU usage.
NV-Segment-CTMR/configs/inference.json Same changes as the CT inference config.

Sequence Diagram

sequenceDiagram
    participant User
    participant MONAI as MONAI Bundle
    participant prepHF as prepare_huggingface_checkpoint
    participant HF as Hugging Face Hub
    participant Disk as Local Filesystem

    User->>MONAI: monai.bundle run (inference.json)
    MONAI->>prepHF: initialize[0]: prepare_huggingface_checkpoint(repo_id, ckpt_file, checkpoint_path, counter_file)
    prepHF->>HF: "hf_hub_download(config.json, force_download=True)"
    HF-->>prepHF: config.json cached path (HF counter touched)
    alt models/model.pt missing
        prepHF->>HF: hf_hub_download(vista3d_pretrained_model/model.pt)
        HF-->>prepHF: cached .pt path
        prepHF->>Disk: symlink_to(cached .pt) OR shutil.copy2
        Disk-->>prepHF: models/model.pt ready
    end
    prepHF-->>MONAI: str(local_path)
    MONAI->>MONAI: "set_determinism(seed=123)"
    MONAI->>Disk: CheckpointLoader(models/model.pt)
    MONAI->>MONAI: evaluator.run()

    Note over MONAI,Disk: Multi-GPU: non-rank-0 workers skip prepHF early,<br/>dist.barrier() ensures rank 0 finishes before CheckpointLoader
Loading

Reviews (1): Last reviewed commit: "Fix pre-commit lint for inference PR" | Re-trigger Greptile

Comment on lines +79 to +82
try:
local_path.symlink_to(checkpoint_path)
except OSError:
shutil.copy2(checkpoint_path, local_path)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Broken symlink not cleared before shutil.copy2 fallback

If the HF cache has been cleaned up (e.g. huggingface-cli delete-cache), local_path can be a broken symlink: local_path.exists() returns False (it follows the symlink), so the code proceeds to download and then calls symlink_to, which raises FileExistsError because the symlink inode still exists. The OSError catch then attempts shutil.copy2(checkpoint_path, local_path), but on Linux this writes through the broken symlink to its (now-missing) target — if the target's parent directory was also removed, this raises FileNotFoundError, leaving the broken symlink in place and inference stuck.

Add if local_path.is_symlink() and not local_path.exists(): local_path.unlink() before the symlink_to call to clear the stale symlink first. The same issue exists in NV-Segment-CTMR/scripts/huggingface_download.py.

Comment on lines +6 to +11
def _is_rank_zero() -> bool:
for name in ("RANK", "LOCAL_RANK", "SLURM_PROCID"):
value = os.environ.get(name)
if value not in (None, "", "0"):
return False
return True

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Multi-node SLURM + torchrun: one env-var being non-zero masks global rank 0

_is_rank_zero returns False if any of RANK, LOCAL_RANK, or SLURM_PROCID is not in (None, "", "0"). In a multi-node SLURM launch that wraps torchrun, it is possible for SLURM_PROCID to be set to the SLURM task ID while RANK/LOCAL_RANK are also set. If those values mismatch (e.g., SLURM assigns non-zero SLURM_PROCID to the PyTorch rank-0 process), the function incorrectly returns False for rank 0, meaning no process performs the HF download, and the barrier in mgpu_inference.json proceeds with models/model.pt still missing, causing all ranks to crash at CheckpointLoader. The same logic is duplicated in the CTMR variant.

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