Skip to content

downcast MiniMax-H3 position_ids at the device transfer on fp64-less backends - #14767

Open
SammyTourani wants to merge 1 commit into
huggingface:mainfrom
SammyTourani:fix/issue-14639
Open

SammyTourani wants to merge 1 commit into
huggingface:mainfrom
SammyTourani:fix/issue-14639

Conversation

@SammyTourani

@SammyTourani SammyTourani commented Sep 14, 2026

Copy link
Copy Markdown

Fixes #14639

One file, 9 added / 3 removed lines. Both .to(device) sites now pass the dtype through
maybe_adjust_dtype_for_device from diffusers.utils.torch_utils:

position_ids_dtype = maybe_adjust_dtype_for_device(position_ids.dtype, device)
block_state.position_ids = position_ids.to(device, position_ids_dtype)

Why this shape rather than the torch.device(device).type == "mps" check the issue proposes:

  • It is the codebase's own idiom for exactly this problem — transformer_flux.py:512,
    transformer_anyflow_far.py:48, unet_2d_condition.py:857, pipeline_pixart_alpha.py:922,
    controlnet_sparsectrl.py:608, auraflow_transformer_2d.py:438, pipeline_flux2_klein.py:408.
  • The repo's own agent guide prescribes it by name: .ai/references/models.md, gotcha Add UNet for Latent Diffusion #5
    ("Only if float32 visibly degrades output, use the maybe_adjust_dtype_for_device
    helper ... Never leave an unconditional torch.float64 in the model").
  • It covers NPU and Neuron too (_FP64_UNSUPPORTED_DEVICES = {"mps", "npu", "neuron"}),
    which a hand-rolled MPS check does not.

The float64 construction is untouched, and no device's numerics change — including MPS.
The issue's precision table (worst case 1.6e-4 rad) is pessimistic: the grid's fp64
precision never reaches the model at all, because MiniMaxH3RotaryPosEmbed.forward
(transformer_minimax_h3.py:95) casts position_ids to float32 as its first statement, on
every device. position_ids has no other consumer — I grepped the whole minimax_h3
package; outside before_denoise.py it only appears in docstrings. So the downcast just
moves a cast that already happens one step earlier. Measured on the largest realistic grid
(64×64 latent frame at patch 2, 102 latent frames, 4000 audio latents, max |coord| 5023):

cos bit-identical: True     max abs diff: 0.0
sin bit-identical: True     max abs diff: 0.0

And on cpu/cuda maybe_adjust_dtype_for_device(torch.float64, device) returns torch.float64,
so those paths are byte-for-byte the code that was there before.

I added no test. Reasoning, which a reviewer may disagree with (see below): the existing
suite already covers this on MPS, because the shared mixins build the pipeline with
.to(torch_device) and tests/testing_utils.py:116 resolves torch_device to "mps"
automatically on Apple silicon — and .github/workflows/push_tests_mps.yml runs pytest tests/
on a macOS runner. The before/after numbers below are that suite. A test that only ass

Verification

Environment: .venv with torch 2.14.0 (MPS available), transformers 5.17.0, torchvision
0.29.0, torchaudio 2.14.0, accelerate, peft. torchvision and torchaudio were missing at
first and caused unrelated failures; both are needed for this suite to run at all.

1. Full suite on MPS, without the patch (baseline, git stashed):

Command:

python -m pytest tests/modular_pipelines/minimax_h3/ -q -p no:randomly -n 4 --timeout=1800

Result:

11 failed, 90 passed, 10 skipped, 225 warnings, 23 errors in 299.12s (0:04:59)

286 occurrences of Cannot convert a MPS Tensor to float64 in that log. Broken tests include
TestMiniMaxH3ModularPipelineFast::test_inference_is_not_nan,
TestMiniMaxH3Ref2VAModularPipelineFast::test_inference_is_not_nan,
::test_float16_inference on both, both Loading::test_save_from_pretrained, and most of
the LoRA class.

2. Same command, same machine, with the patch:

Command:

python -m pytest tests/modular_pipelines/minimax_h3/ -q -p no:randomly -n 4 --timeout=1800

Result:

4 failed, 116 passed, 14 skipped, 249 warnings in 535.19s (0:08:55)

The 4 remaining failures are all Memory::test_*_auto_cpu_offload* and are an unrelated,
pre-existing MPS gap — components_manager.py:749 raises
NotImplementedError: enable_auto_cpu_offload() relies on the mem_get_info() method. It's not implemented for mps. because torch.mps has no mem_get_info. They fail the same way
with and without this patch and have nothing to do with position_ids.

3. Full suite on CPU with the patch (no regression on the default path):

Command:

DIFFUSERS_TEST_DEVICE=cpu python -m pytest tests/modular_pipelines/minimax_h3/ -q -p no:randomly -n 6 --timeout=1800

Result:

108 passed, 26 skipped, 228 warnings in 266.66s (0:04:26)

4. All three workflows end to end on MPS (scratch script, tiny checkpoint, 124 frames,
32×32, 2 steps) — fails at before_denoise.py:444 / :768 before, passes after:

OK t2va:  video=(1, 124, 3, 32, 32) audio=(1, 2, 828)
OK fl2va: video=(1, 124, 3, 32, 32) audio=(1, 2, 828)
OK ref2va: video=(1, 124, 3, 32, 32) audio=(1, 2, 828)

5. Style / consistency:

ruff 0.9.10 check           -> All checks passed!
ruff format --check         -> 1 file already formatted
python utils/check_copies.py  -> clean
python utils/check_dummies.py -> clean
python utils/check_ai.py      -> clean
doc-builder style ... --max_len 119 --check_only -> clean

…backends

Both layout steps build position_ids in float64 on CPU and then move it to
the execution device with a plain .to(device), which raises on MPS. Pass the
dtype through maybe_adjust_dtype_for_device so the transfer lands as float32
on mps/npu/neuron and is unchanged everywhere else. The fp64 construction is
untouched, and MiniMaxH3RotaryPosEmbed.forward already casts position_ids to
float32, so no device sees different rotary angles.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MiniMax-H3: float64 rotary position grid cannot be moved to MPS (Cannot convert a MPS Tensor to float64)

1 participant