fix: answer the bfloat16 dtype question without allocating - #310
Merged
Merged
Conversation
The probe built a one-element bfloat16 tensor on the target device, so a dtype query took 256 bytes of device memory and failed outright when a loaded GPU could not spare them. A zero-element tensor gets the same verdict from the same size-independent check, and asks the allocator for nothing. Also applied to the three FLUX VAE loader sites. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
lstein
requested review from
JPPhoto,
Pfannkuchensack and
blessedcoolant
as code owners
September 22, 2026 01:35
7 tasks
lstein
enabled auto-merge
September 22, 2026 11:03
lstein
disabled auto-merge
September 22, 2026 11:56
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
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
TorchDevice.choose_bfloat16_safe_dtypedecided which dtype to use by building a one-element bfloat16 tensor on the target device. That put a real allocation between a caller and its dtype: on a loaded GPU the allocation fails, and since onlyTypeErrorwas caught, the failure escaped as aRuntimeErrorfrom a function that had been asked a question about dtypes, before the caller had requested any memory of its own.This is what turned a memory-starved macOS CI runner red:
The probe now builds a zero-element tensor. It gets the same verdict from the same check, and asks the allocator for nothing:
empty_mpswith aTORCH_CHECK_TYPEon the dtype argument, above theallocator->allocate()call and with no reference to the element count — soTypeErrorstill fires on macOS 13 and the float32 fallback is unchanged (verified in PyTorch v2.7.1 and v2.4.0 source, at the versionpyproject.tomlpins for darwin).MPSAllocator::allocateisnbytes > 0 ? _getAllocImpl().malloc(...) : nullptr, so zero bytes never reaches Metal.torch.empty(0, …)allocates 0 bytes and itsdata_ptris null, against 512 bytes plus a 2 MiB reserved segment for the old probe. It is also ~30x faster per call on GPU, which matters on a loader path that calls it once per component.The same allocating probe appears verbatim at three FLUX VAE loader sites (
flux.py:158,:184,:284) — on a load path, where VRAM is tightest — and is fixed the same way, so the repo answers this question one way.No dtype changes on any device: bfloat16 stays bfloat16 on CUDA/ROCm, CPU, MPS on macOS 14+ and XPU; macOS 13 still gets float32.
Related Issues / Discussions
Companion to #309, which reduces the memory pressure on the macOS runner. This PR removes the dependence on that memory in the first place; #309 is still worth having, since 6.70GB of workers on a 7GB runner has no headroom for anything else.
Merging this into main fixes the failing macOS jobs on the open LTX-2 PRs (#305 and the rest of the stack) with no change to their tests.
QA Instructions
pytest tests/backend/util/ tests/backend/model_manager/load/ …— 1423 passed, 150 skipped, 6 xfailed. ruff check and format clean.Tried to allocate 256 bytes on shared poolerror on main's probe, and passes with this change applied and nothing else. The test itself is untouched.RuntimeError. Each is caught by at least one of the new tests.Review
Two independent read-only reviews, both adversarial. Both found material problems in the first draft, which the diff now reflects:
RuntimeErrorand returned bfloat16. That was wrong twice over: once the probe stopped allocating, the branch could no longer catch the OOM it was written for, and what it could still catch was a device that cannot be reached at all — an out-of-range ordinal, or a backend this build lacks. Those used to raise immediately and by name; the draft turned them into a plausible dtype and a failure much later inside a loader. The branch is gone; a test now pins that an unreachable device still raises.TypeErrorfallback test gained anxpucase: that arm is theelseof the CUDA branch, so it covers XPU too.Known limitations, stated rather than papered over: the CUDA → float16 arm is preserved but is not known to be reachable (torch does not reject bfloat16 as a storage dtype on any CUDA arch); the new tests capture the constructors a probe would plausibly use, so a probe spelled some other way records nothing rather than failing. Pre-existing and out of scope:
test_devices.py'schoose_anima_inference_dtypetests setconfig.precisionon the global singleton without restoring it, which makes two unrelated tests in that file order-dependent.Checklist
What's Newcopy (if doing a release after this PR)🤖 Generated with Claude Code