Guard float64 against fp64-less devices in the Helios schedulers - #14375
Open
4ktLuffy wants to merge 1 commit into
Open
Guard float64 against fp64-less devices in the Helios schedulers#143754ktLuffy wants to merge 1 commit into
4ktLuffy wants to merge 1 commit into
Conversation
Both Helios schedulers built float64 tensors and moved them straight to the target
device. In the multi-stage branch `timesteps` and `sigmas` both come from
`np.linspace`, which is float64, so `set_timesteps(..., device="mps")` raised
TypeError: Cannot convert a MPS Tensor to float64 dtype as the MPS framework
doesn't support float64.
`HeliosPyramidPipeline` calls `set_timesteps(..., device=device)`, so on Apple
Silicon the pipeline failed at the first scheduler call, before any model ran.
`HeliosDMDScheduler.convert_flow_pred_to_x0` had the same problem separately: it
calls `.double()` on four tensors that already live on the device.
All three sites now route the dtype through `maybe_adjust_dtype_for_device`, the
helper the rest of the codebase already uses for this (53 call sites). It downcasts
float64 to float32 only on devices listed in `_FP64_UNSUPPORTED_DEVICES` — mps, npu
and neuron — and is a no-op everywhere else, so CPU and CUDA keep full float64
precision.
Verified: all three sites raise TypeError on mps before this change and succeed
after it, while CPU results are bit-identical across `set_timesteps` for both
schedulers and `convert_flow_pred_to_x0`.
Adds tests/schedulers/test_scheduler_helios.py, the first test coverage for these
schedulers. Both tests fail on main and pass with this change.
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.
What this fixes
Fixes #14367.
Both Helios schedulers build
float64tensors and move them straight to the target device. In the multi-stage branch,timestepsandsigmasboth come fromnp.linspace, which isfloat64, so on a device that cannot hold it the move raises:HeliosPyramidPipelinecallsset_timesteps(..., device=device), so on Apple Silicon this fails at the first scheduler call, before any model runs.Three sites, all reproducible on
main(0.40.0.dev0):scheduling_helios.pyset_timestepsHeliosSchedulerscheduling_helios_dmd.pyset_timestepsHeliosDMDSchedulerscheduling_helios_dmd.pyconvert_flow_pred_to_x0—.double()on four on-device tensorsHeliosDMDSchedulerThe change
All three route the dtype through
maybe_adjust_dtype_for_device, which this codebase already uses for exactly this (53 call sites). It downcastsfloat64only on devices in_FP64_UNSUPPORTED_DEVICES—mps,npu,neuron— and is a no-op elsewhere, so CPU and CUDA keep fullfloat64precision.In
set_timestepsthe cast happens before the move rather than after, since the failing operation is the move.Verification
HeliosScheduler.set_timesteps(device="mps")TypeErrorfloat32HeliosDMDScheduler.set_timesteps(device="mps")TypeErrorfloat32HeliosDMDScheduler.convert_flow_pred_to_x0on mpsTypeErrorfloat64float64Full
tests/schedulers/: 6 failed / 979 passed with this change, versus 8 failed / 977 passed without it — the same six pre-existing local MPS failures either way, plus the two new tests.Tests
tests/schedulers/test_scheduler_helios.pyis new; these schedulers had no test coverage. Both tests fail onmainand pass with this change.They assert against
torch_device, comparing the resulting dtype tomaybe_adjust_dtype_for_device(torch.float64, torch_device). On a CPU or CUDA runner that assertsfloat64is left alone; on an fp64-less device it asserts the downcast happened. So the tests are meaningful on CI without requiring Apple hardware, though only an mps/npu/neuron runner exercises the actual regression.Overlap with #14071
@Shreyas-jk opened #14071 in June, which fixes the third site (
convert_flow_pred_to_x0). I did not spot it when I filed #14367 — apologies for the duplication, that was my miss.The two differences, so you can pick rather than guess:
convert_flow_pred_to_x0upcast only. The twoset_timestepssites are untouched by it, and those are the ones that breakHeliosPyramidPipelinefirst — it callsset_timesteps(..., device=device)before any model runs, so on Apple Silicon the failure happens there, not inconvert_flow_pred_to_x0.torch.float32 if device.type == "mps" else torch.float64. Routing throughmaybe_adjust_dtype_for_deviceinstead picks upnpuandneuronfrom_FP64_UNSUPPORTED_DEVICESas well, and keeps this file consistent with the other 53 call sites.I am happy to drop my third hunk and rebase on top of #14071 if you would rather merge that one first — its author got there before me and the credit for that site is theirs either way. Just say which you prefer.