Skip to content

Commit f979a26

Browse files
Longyun Shenclaude
andcommitted
Fix neighbor_list_from_ase(compute_distances=True) crashing on periodic structures
_compute_distances multiplied the integer unit-shift matrix returned by the neighbor-list kernel with the float cell; integer x float matmul is not defined in torch, so the DEFAULT arguments of neighbor_list_from_ase raised 'expected mat1 and mat2 to have the same dtype' on any structure with periodic boundary conditions. Non-periodic structures dodge the path (unit_shifts is None), which is presumably how it survived smoke tests. Cast the shifts to the position dtype at the point of use. The new test runs the default path on a periodic rocksalt supercell and checks the returned distances against a float64 recomputation (1e-4) and the cutoff, not merely the absence of a crash. While auditing this path we also cross-checked the neighbor list itself: on a dense triclinic 1,248-atom cell at 5.0 A the returned edge set is bit-identical (shift vectors included) to an exact O(N^2) minimum-image reference -- 25,242 edges, zero missing, zero spurious. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 25b3a29 commit f979a26

2 files changed

Lines changed: 43 additions & 1 deletion

File tree

src/matgl/ext/_alchmtk.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,11 @@ def _compute_distances(
217217
"""Compute the distances between the source and destination atoms."""
218218
vectors = positions[dst_id] - positions[src_id]
219219
if cell is not None and unit_shifts is not None:
220-
vectors += unit_shifts @ cell
220+
# unit_shifts arrives as an integer tensor from the neighbor-list
221+
# kernel; integer x float matmul raises "expected mat1 and mat2 to
222+
# have the same dtype", so every periodic call with
223+
# compute_distances=True (the default) crashed here.
224+
vectors += unit_shifts.to(positions.dtype) @ cell
221225
return torch.linalg.norm(vectors, dim=1)
222226

223227

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
"""Tests for the ``matgl.ext._alchmtk`` neighbor-list helpers."""
2+
3+
from __future__ import annotations
4+
5+
import pytest
6+
import torch
7+
8+
pytest.importorskip("nvalchemiops", reason="nvalchemi-toolkit-ops required")
9+
10+
from ase.build import bulk
11+
12+
pytestmark = pytest.mark.skipif(not torch.cuda.is_available(), reason="the nvalchemiops path needs CUDA")
13+
14+
15+
def test_neighbor_list_from_ase_computes_distances_on_periodic_cells():
16+
"""``compute_distances=True`` (the default) must work under PBC.
17+
18+
``_compute_distances`` did ``unit_shifts @ cell`` with the integer shift
19+
matrix the neighbor-list kernel returns, and integer x float matmul
20+
raises ``RuntimeError: expected mat1 and mat2 to have the same dtype``.
21+
Non-periodic structures dodge the path (``unit_shifts is None``), which
22+
is how it survived smoke tests. Beyond surviving, the values must match
23+
a float64 recomputation from the returned tensors.
24+
"""
25+
from matgl.ext._alchmtk import neighbor_list_from_ase
26+
27+
atoms = bulk("NaCl", "rocksalt", a=5.64).repeat((3, 3, 3))
28+
src, dst, dist, shifts, pos, _ = neighbor_list_from_ase(atoms, cutoff=5.0, compute_distances=True, device="cuda")
29+
30+
assert dist is not None
31+
assert dist.shape[0] == src.shape[0]
32+
cell = torch.as_tensor(atoms.get_cell().array, dtype=torch.float64, device=pos.device)
33+
ref = torch.linalg.norm(
34+
pos[dst.long()].double() + shifts.double() @ cell - pos[src.long()].double(),
35+
dim=1,
36+
)
37+
assert torch.allclose(dist.double(), ref, atol=1e-4)
38+
assert float(dist.max()) <= 5.0 + 1e-4

0 commit comments

Comments
 (0)