Skip to content

Cast float64 targets before the device move in the context-cache path - #74

Open
manjunathshiva wants to merge 1 commit into
google-research:mainfrom
manjunathshiva:fix-mps-float64-cache-context
Open

Cast float64 targets before the device move in the context-cache path#74
manjunathshiva wants to merge 1 commit into
google-research:mainfrom
manjunathshiva:fix-mps-float64-cache-context

Conversation

@manjunathshiva

Copy link
Copy Markdown

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_pytorch has the identical pattern, and the two paths are disjoint — with cache_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:

TypeError: Cannot convert a MPS Tensor to float64 dtype as the MPS framework
doesn't support float64. Please use float32 instead.

Repro (crashes at classifier_and_regressor.py:2067 on current main):

import numpy as np
from tabfm import TabFMRegressor
from tabfm.src.pytorch import tabfm_v1_0_0

model = tabfm_v1_0_0.load(model_type="regression", device="mps")
rng = np.random.default_rng(0)
X = rng.random((100, 5), dtype=np.float32)
y = rng.random(100)  # float64 — numpy's default float dtype
reg = TabFMRegressor(model=model, n_estimators=2, cache_context=True)
reg.fit(X[:80], y[:80])  # TypeError during cache build

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=True on 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 TabFMClassifier and TabFMRegressor run on device="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).

_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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant