Cast float64 targets before the device move in the context-cache path - #74
Open
manjunathshiva wants to merge 1 commit into
Open
Cast float64 targets before the device move in the context-cache path#74manjunathshiva wants to merge 1 commit into
manjunathshiva wants to merge 1 commit into
Conversation
_build_context_cache_pytorch moved y to the model device before casting float64 down to float32. MPS rejects float64 tensors at transfer time, so fit(cache_context=True) crashed on Apple Silicon for any float64 target array (numpy's default float dtype). Same root cause as google-research#68 / google-research#69, which cover the uncached _predict_step_pytorch path; this fixes the remaining site. CPU/CUDA are unaffected: they accept float64 and the later cast made the ordering unobservable there. Adds a regression test that fits a small regressor with cache_context=True on an MPS model with float64 targets; it fails with the TypeError against the previous ordering and skips where MPS is unavailable.
manjunathshiva
requested review from
abhidas,
erzel,
rajatsen91,
siriuz42,
tamannarayan and
weihaokong
as code owners
July 20, 2026 04:19
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.
Same root cause as #68, second call site: #69 fixes the float64→float32 cast ordering in
_predict_step_pytorch(the uncached predict path), but_build_context_cache_pytorchhas the identical pattern, and the two paths are disjoint — withcache_context=True, prediction goes through prefill/decode and never calls_predict_step_pytorch. So on Apple Silicon (device="mps"),fit(cache_context=True)still crashes after #69:Repro (crashes at
classifier_and_regressor.py:2067on current main):The fix mirrors #69: do the existing conditional cast on the CPU tensor, then move. The cast stays conditional for the same reasons noted there (int64 classifier targets must keep their dtype). CPU/CUDA behavior is unchanged — those backends accept float64 and the later cast made the ordering unobservable.
The regression test fits a small regressor with
cache_context=Trueon an MPS model with float64 targets. It fails with the TypeError above against the previous ordering, and skips where MPS is unavailable (same trade-off as #69; on CPU the old and new orderings are indistinguishable, so a CPU test would not guard anything).Verified end-to-end on Apple Silicon (M-series, torch 2.13.0) with the real v1.0.0 checkpoints: with this fix plus #69, both
TabFMClassifierandTabFMRegressorrun ondevice="mps"with cached and uncached prediction, 100% label agreement vs CPU on breast_cancer (probability deltas ≤3e-2, consistent with bf16 kernel differences), regression deltas ~2% of target std on diabetes, and ~16–22× speedups over CPU (e.g. classifier predict 38.4s → 2.4s, cached fit 35.9s → 1.6s).