Skip to content

Make node_energy Mac Silicon MPS-compatible (avoid hardcoded float64) - #1505

Merged
aacostadiaz merged 2 commits into
ACEsuit:mainfrom
aakolganov:patch-1
Jul 15, 2026
Merged

Make node_energy Mac Silicon MPS-compatible (avoid hardcoded float64)#1505
aacostadiaz merged 2 commits into
ACEsuit:mainfrom
aakolganov:patch-1

Conversation

@aakolganov

@aakolganov aakolganov commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

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_SUMMARY -->
---

> [!NOTE]
> **Low Risk**
> Single-line behavioral change in ScaleShiftMACE forward; MPS gets float32 node_energy instead of crashing, CPU/CUDA behavior unchanged.
> 
> **Overview**
> Fixes **ScaleShiftMACE** inference on Apple Silicon when tensors live on **MPS**, where PyTorch cannot use float64.
> 
> **`node_energy`** was built by cloning **`node_e0`** and **`node_inter_es`** with **`.double()`**, which crashes on MPS. The forward pass now picks **`energy_dtype`**: **`node_e0.dtype`** on MPS (typically float32) and **`torch.float64`** on CPU/CUDA, then sums the clones in that dtype. **`total_energy`**, forces, and other outputs are unchanged; only the per-atom energy tensors accumulation dtype is device-aware.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 7117d352cccd4c6d7fba5e15777fbc40be66cdf4. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

…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)
@aakolganov aakolganov changed the title Make node_energy Mac Silicon MPS-compatible (avoid hardcoded float64 … Make node_energy Mac Silicon MPS-compatible (avoid hardcoded float64) Jun 22, 2026
@ilyes319

Copy link
Copy Markdown
Contributor

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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.
@aacostadiaz

Copy link
Copy Markdown
Collaborator

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 .double() pattern exists in extensions.py, so those paths would still raise the same TypeError on MPS. Behaviour on CPU/CUDA is unchanged.

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: MACECalculator still fails earlier on MPS when loading a float64 checkpoint, because torch.load(..., map_location=device) (mace/calculators/mace.py) can't materialize float64 weights on mps. Workaround for now is torch.load(path, map_location="cpu").float().to("mps");

@aacostadiaz
aacostadiaz merged commit 22f0809 into ACEsuit:main Jul 15, 2026
25 checks passed
aacostadiaz added a commit that referenced this pull request Aug 5, 2026
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.
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.

3 participants