Make node_energy Mac Silicon MPS-compatible (avoid hardcoded float64) - #1505
Conversation
…in forward) Issue: The Apple-Silicon GPU MPS backend does not support float64, so this raises at inference on device="mps", even when the model is loaded/run in float32: "TypeError: Cannot convert a MPS Tensor to float64 dtype as the MPS framework doesn't support float64. Please use float32 instead." Fix: Keep the float64 on CPU/CUDA (unchanged behaviour) and fall back to the tensor's working dtype only on MPS energy_dtype = node_e0.dtype if node_e0.device.type == "mps" else torch.float64 node_energy = node_e0.clone().to(energy_dtype) + node_inter_es.clone().to(energy_dtype)
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 7117d35. Configure here.
MPS has no float64 support, so hardcoded .double() calls raise at inference on Apple-Silicon GPUs. Replace the inline dtype check with a shared, TorchScript-compatible safe_double() in modules/utils.py and apply it to every .double() call in the forward paths: the node_energy upcast in ScaleShiftMACE and both PolarMACE variants, plus the fukui/charge-density scatter accumulations. Behaviour on CPU/CUDA is unchanged; on MPS tensors keep their working dtype.
|
Thanks @aakolganov for tracking this down and for the fix, much appreciated! I've pushed a small follow-up commit on top of yours that generalizes it: the same hardcoded Verified on Apple Silicon: ScaleShiftMACE and PolarMACE forwards run on mps in float32, and MACE-MP-small through the ASE calculator gives energies/forces matching CPU float32 to ~1e-7. One heads-up, out of scope for this PR: |
Brings the four commits that landed on main into develop: 54b9645 Add control of dispersion coordination cutoff in mace_mp (#1536) e333ae6 LES extension (dipole, quadrupoles, polarizability, etc.) (#1478) 22f0809 Make node_energy Mac Silicon MPS-compatible (#1505) f1c09d8 fix cueq_conv_fusion for LAMMPS (#1475) Three conflicts, all additive, resolved as unions: mace/modules/__init__.py export MACELES alongside the magnetic classes; __all__ already listed both mace/calculators/mace.py keep develop's model_kwargs name, add main's external_field injection and the compute_bec branch mace/modules/extensions.py union of both import sets; every symbol on both sides is used tests/extensions/les/test_maceles.py merged automatically: git followed the rename from the test-suite restructure on develop.
Issue
The Apple-Silicon GPU MPS backend does not support float64, so this raises at inference on
device="mps", even when the model is loaded/run in float32:TypeError: Cannot convert a MPS Tensor to float64 dtype as the MPS framework doesn't support float64. Please use float32 instead.
Fix
Keep the float64 on CPU/CUDA (unchanged behaviour) and fall back to the tensor's working dtype only on MPS: