Commit graph

10 commits

Author SHA1 Message Date
Daniel Han
10f0a03c8f docker/test_locally.sh: fail fast + pin notebook fetch to immutable SHA
Two small fixes:

1. The fallback build-context refresh used `git pull --ff-only | tail`,
   which on this script (set -uo pipefail, no -e) silently masked any
   non-zero exit from pull. A failed refresh would then quietly build
   from a stale clone. Wrap both clone and pull in `if ! ...; then fail`
   so refresh failures abort the run with a clear message.

2. The gpt-oss-20B notebook was fetched from notebooks/main, which is
   mutable. Pin to the current immutable SHA (efe20c9) via NB_REPO_REF
   so reruns of this script don't silently change semantics when
   notebooks/main rolls forward. Override via env when you want to
   verify a newer notebook.
2026-05-27 15:56:30 +00:00
Daniel Han
cceeeb1e1b Address 3 MAJOR review findings on the docker PR
1. Stop leaking secrets via docker run -e VAR=VALUE argv (run.sh, test_locally.sh)

   `docker run ... -e HF_TOKEN=hf_xxx ...` puts the literal token in
   the docker CLI's argv, which is visible to any user on the host
   via `ps auxe` / `/proc/<pid>/cmdline` for the lifetime of the
   process. Switch to the dash-only form `-e HF_TOKEN`, which tells
   docker to read the value from the parent shell's env and never
   appears in argv. Same fix for WANDB_API_KEY and UNSLOTH_LICENSE in
   run.sh and HF_TOKEN in test_locally.sh.

2. Stop stripping numpy/tests/ in the runtime layer (Dockerfile)

   The Dockerfile explicitly upgrades numpy >= 2.4 because numpy 2.2.6
   shipped a stripped wheel where `from numpy._core.tests._natype
   import pd_NA` fails. Numpy 2.4 restores `numpy/_core/tests/`, then
   the existing `find ${VENV} -name tests -exec rm -rf {} +` deleted
   it again -- re-introducing the same broken-import state on the
   deployed image (the build-time verification at line 220 runs
   BEFORE the strip so it passed). Whitelist numpy's tests directories
   from the strip; keep stripping the rest.

3. Align :latest tag gate between merge and smoke-test jobs
   (.github/workflows/docker-publish.yml)

   merge job:       enable = is-default-branch AND unsloth_ref == ''
   smoke-test job:  enable = is_default_branch only

   On `workflow_dispatch`, `github.event.inputs.unsloth_ref` defaults to
   "main" (not ""), so the merge step skipped `:latest` but the smoke
   step still emitted `:latest` as tags[0]. The smoke step then
   `docker pull`-ed a prior `:latest` from Docker Hub instead of the
   image just merged -- so the smoke test verified the OLD image, not
   the new one. Copy the merge step's exact `enable=` expression into
   the smoke-test step so the two stay byte-identical and a workflow_
   dispatch run validates whatever was actually merged.
2026-05-25 13:36:56 +00:00
danielhanchen
0d574d8161 Address reviewer-2 findings on PR #5748
Round-2 of the 12-persona reviewer.py pass found 17 issues. Address the
P1s + the regression-class P2s in this commit; the remaining nits are
left for a follow-up cleanup pass.

1. unsloth/_gpu_init.py: the `NVIDIA_VISIBLE_DEVICES in os.environ` check
   triggered for every NVIDIA-runtime container including `--gpus all`
   (NVIDIA_VISIBLE_DEVICES=all is the default). Gate strictly on a
   non-special device list. Also drop the precondition that the env var
   was absent: if the user already pinned TORCHINDUCTOR_COMPILE_THREADS=1
   we should still plant the UNSLOTH_FORCE_SINGLE_COMPILE_WORKER sentinel
   so the zoo-side patch knows to preserve the forcing.

2. unsloth/_gpu_init.py: after the post-`import unsloth_zoo` reassertion,
   monkey-patch `unsloth_zoo.temporary_patches.common.determine_compile_threads`
   to return 1, so any later `torch.compile` call that rebuilds the
   options dict still sees the single-worker forcing even if a downstream
   patch_torch_compile pops the env var again.

3. docker/Dockerfile: torchaudio==2.11.0 mismatched the torch==2.10.0
   release pairing; pin to 2.10.0 so the ABI is correct and the audio
   stack matches torch/cu128.

4. docker/Dockerfile: drop `12.1+PTX` from TORCH_CUDA_ARCH_LIST. The
   cu128 toolkit compiler does not know about compute_121; the trailing
   PTX entry forced nvcc to emit a `sm_121` gencode that breaks any
   in-container source builds.

5. docker/smoke_test.py: the device-capability floor said `cap[0] < 8`,
   rejecting Turing (sm_75) while the Dockerfile + entrypoint advertise
   sm_75 as supported. Lower the smoke floor to sm_75 and print a hint
   that bf16 is not available on Turing.

6. docker/run.sh: `-it` is unconditional; CI / non-TTY invocations died
   with "the input device is not a TTY". Probe `[ -t 0 ] && [ -t 1 ]`
   first. Also remove `set -x` which echoed the forwarded HF_TOKEN /
   WANDB_API_KEY / UNSLOTH_LICENSE values to stdout.

7. docker/test_locally.sh: `-e HF_TOKEN="${HF_TOKEN:-}"` either pasted
   the secret verbatim into the process arg list or shadowed any
   in-container value with an empty string. Forward conditionally.

8. .github/workflows/docker-publish.yml: gate `latest` on default branch
   AND on `unsloth_ref` not being overridden via workflow_dispatch.
   Otherwise a maintainer testing a feature SHA from main could overwrite
   `:latest` with non-main source.

9. docker/Dockerfile.studio: add an `UNSLOTH_STUDIO_REF` build-arg so
   the Studio companion image is pinned to a known unsloth ref instead
   of cloning `main` whenever it builds.
2026-05-24 15:24:20 +00:00
Daniel Han
e7cfceadab Add linux/arm64 (DGX Spark / Grace) support via QEMU at build time
Make the docker image multi-arch so DGX Spark (GB10, sm_121, aarch64) and
the Grace-Hopper / Grace-Blackwell SoCs (GH200 arm64, GB200 arm64) pull a
natively-built arm64 child from the same manifest. Runtime emulation is
NOT involved -- QEMU is used only for the cross-compile step on x86_64
CI runners; consumers on aarch64 hosts get a normal arm64 image and CUDA
works as on any other host.

Dockerfile:
  * ARG TARGETARCH; switch unsloth extras between cu128-ampere-torch2100
    (amd64, with xformers) and huggingface (arm64, no xformers -- there
    is no cu128 aarch64 xformers wheel as of 0.0.34, so we fall back to
    Unsloth's native SDPA path; ~5-10% slowdown but functionally complete).
  * Build-time torch._C._cuda_getArchFlags() assertion: amd64 still
    requires sm_120, arm64 accepts sm_120 or sm_121.
  * Same TORCH_CUDA_ARCH_LIST on both arches; nvcc emits whatever's listed.

docker/setup_qemu.sh (new):
  One-time host setup -- registers binfmt_misc handlers via
  tonistiigi/binfmt and creates a 'unsloth-multiarch' docker-container
  buildx builder. Required only on x86_64 build hosts targeting arm64.

docker/test_locally.sh:
  --platform amd64|arm64 flag. Cross-builds verify QEMU is registered,
  then build through the in-image arch-flags assertion. Smoke + notebook
  blocks auto-skip when image arch != host arch (CUDA cannot run under
  user-space QEMU + nvidia-container-toolkit cannot bridge a QEMU guest
  to a real GPU).

.github/workflows/docker-publish.yml:
  platforms: linux/amd64,linux/arm64 (single manifest, two children).
  Timeout bumped 60 -> 150 min for the slower arm64-under-QEMU leg.
  docker/setup-qemu-action@v3 with platforms: arm64 (was implicit before).
2026-05-24 10:34:59 +00:00
Daniel Han
391532c031 test_locally.sh: skip notebook install cells, strip stray jupyter magic
The previous nbformat-based conversion dumped raw cell.source for every
code cell. The gpt-oss-20B notebook's first cell uses Jupyter !shell
magic to install dependencies:

  !pip install --upgrade -qqq uv
  !uv pip install -qqq ... \
  git+https://github.com/triton-lang/triton.git@0add68... ...

Dumped verbatim, the `@0add68...` token tripped the Python parser with
"SyntaxError: invalid decimal literal" before training could even start.

The container already has unsloth, triton, transformers, etc. baked in,
so we don't need the notebook's install cell. Skip any cell whose source
contains pip/install markers, and comment out stray !cmd / %magic lines
in any other cells. Then assert nb.py parses with ast.parse() before
trying to run it -- catches conversion failures up front instead of at
training time.

Reproduces on RTX PRO 6000 Blackwell (sm_120, fresh Docker 29.2.1
host) where the previous conversion produced an invalid nb.py.
2026-05-24 10:11:46 +00:00
Daniel Han
8344fa0a56 test_locally.sh: use nbformat directly, drop fragile jupyter nbconvert call
`jupyter nbconvert --to script nb.ipynb --output nb 2>/dev/null` was
silently exiting 0 without producing the output file in some
environments (likely because jupyter/jupyter_core wasn't on PATH or
nbconvert's --output handling differed across versions). The 2>/dev/null
hid the underlying error, and `set -e` did not catch the missing-output
case because nbconvert itself returned 0.

Switch to a direct nbformat-based conversion:

  pip install -q nbformat
  python -c "import nbformat; nb=nbformat.read('nb.ipynb', as_version=4);
             code='\n\n'.join(c.source for c in nb.cells if c.cell_type == 'code')
             open('nb.py','w').write(code + '\n')"

Smaller dep set, no shell-out to a jupyter wrapper script, and an
explicit `test -s nb.py` afterwards catches any silent failure
before downstream steps try to read the file.

Reproduces the failure on RTX PRO 6000 Blackwell (sm_120, docker
29.2.1, ubuntu 24.04) where nbconvert's CLI silently no-op'd.
2026-05-24 10:00:37 +00:00
Daniel Han
23a5b43180 test_locally.sh: pre-flight check for docker daemon connectivity
If the user is not in the 'docker' group, every docker command after the
pre-flight returns "permission denied while trying to connect to the Docker
daemon socket at /var/run/docker.sock". This used to surface as a confusing
buildx failure mid-Block-2, but the actual problem is a host permissions
issue that's settable up front.

Detect by running 'docker info' and checking its exit code (not just grep
on its output -- a permission failure prints to stderr and returns non-zero,
so the old grep-based check was a silent skip).

Also clarify the nvidia-runtime WARN: on Docker 28+ with CDI mode this is
a false positive most of the time. The real GPU-attach test is the smoke
run in Block 3a, where the container entrypoint catches missing GPUs with
an actionable message.
2026-05-24 07:50:30 +00:00
Daniel Han
56d2701a38 test_locally.sh: require docker buildx, no legacy fallback
Docker 28 removed the legacy image builder entirely. Setting
DOCKER_BUILDKIT=1 no longer falls back to a builtin builder -- it
delegates to buildx, which then errors out if buildx isn't installed:

  ERROR: BuildKit is enabled but the buildx component is missing
         or broken.

The Ubuntu docker.io package omits buildx by default, so users on
that path hit this immediately. Detect missing buildx up front and
print exact install commands for apt / dnf / manual binary instead
of attempting a fallback that cannot work.
2026-05-24 07:24:14 +00:00
Daniel Han
f7b34793f2 test_locally.sh: use docker buildx (or DOCKER_BUILDKIT=1) for the build
The Dockerfile uses BuildKit-only features (the # syntax=docker/dockerfile:1.7
parser directive and RUN ... <<'PY' heredocs added in dockerfile 1.3+). The
legacy builder rejects the --progress flag at the CLI level and would fail
later at the heredocs anyway.

Detect docker buildx and use it when available (preserves --progress=plain
output). Otherwise fall back to plain `docker build` with DOCKER_BUILDKIT=1
exported, which gets the BuildKit features without buildx's nicer formatting.

Reproduces the failure path seen on Docker 28.2.2 without buildx installed:
  unknown flag: --progress
  ERROR docker build exited 125
2026-05-24 07:21:56 +00:00
Daniel Han
acbb16c8a1 Add docker/test_locally.sh: one-shot end-to-end Docker validation
Single bash script that runs the full validation flow against the image:

  1. Host pre-flight: docker version, nvidia-smi, nvidia-container-toolkit
     runtime registered with docker.
  2. Build the image (auto-detects the build context -- current dir,
     docker/ subdir, or clones the docker-blackwell-build branch into
     /tmp/unsloth-pr/).
  3a. Smoke test: 5-step LoRA on Llama-3.2-1B-Instruct-bnb-4bit.
  3b. Real workload: gpt-oss-20B fine-tuning notebook from
      unslothai/notebooks, patched to max_steps=10, with the three
      pre-train demo generations dropped for brevity. Auto-installs
      triton_kernels at the SHA the upstream notebook pins for MXFP4.

All output is teed to /tmp/unsloth-docker-test/ (or --log-dir).

Usage:
  bash docker/test_locally.sh                  # full run, ~15 min
  bash docker/test_locally.sh --skip-notebook  # blocks 1-3a only, ~3 min
  bash docker/test_locally.sh --skip-build     # reuse existing TAG
  TAG=my:tag HF_TOKEN=hf_xxx bash docker/test_locally.sh

Each block fails fast with the exact log path to paste back.
2026-05-24 07:14:47 +00:00