Skip to content

Honor calculator dtype in GraphConverter.convert - #174

Open
shaneraphel wants to merge 1 commit into
microsoft:mainfrom
shaneraphel:graph-convert-honors-dtype-171
Open

shaneraphel wants to merge 1 commit into
microsoft:mainfrom
shaneraphel:graph-convert-honors-dtype-171

Conversation

@shaneraphel

@shaneraphel shaneraphel commented Sep 17, 2026 •

Copy link
Copy Markdown

GraphConverter.convert is the current graph I/O used by MatterSimCalculator.calculate when direct_graph is off. It built atom_pos, cell, pbc_offsets, and the optional energy/force/stress labels with torch.FloatTensor, which is float32 regardless of the calculator's requested dtype.

calculate() later upcasts floating tensors when dtype is float64. That upcast cannot recover digits already rounded away. A rigid translation of a water molecule by 1000 Å in a 4000 Å cell therefore changes the stored coordinates at the 1e-5 Å scale, and the issue's energy/force comparison is the physical consequence of that rounding.

The direct-graph sibling (_build_graph_direct) already constructs positions and cell with dtype=self.dtype. This change puts the current converter on the same contract: convert reads kwargs["dtype"] (default still float32) for every floating tensor it builds, and calculate() passes dtype=self.dtype into build_dataloader, which already forwards **kwargs into convert.

Default float32 inference is unchanged. BatchGraphConverter is a separate construction path and is not rewritten here.

Paired observations on the issue's H2O geometry (no checkpoint):

  • current FloatTensor max |Δ| vs ASE float64 positions: 1.2681640612299816e-05 Å
  • this change tensor(..., dtype=float64) max |Δ|: 0
  • this change default float32 equals current FloatTensor
  • current translation component spread: 3.0517578125e-05 Å; this change float64 spread: 0

Reproducer from the issue still applies after this change: construct MatterSimCalculator(..., dtype="float64"), compare energy and forces of a structure and its rigid translate. The two should now agree at float64 scale on the current dataloader path.

The leftover path builds atom_pos, cell, pbc_offsets and labels with
torch.FloatTensor, which is float32 even when MatterSimCalculator is
constructed with dtype="float64". The later upcast cannot recover the
rounded coordinates, so a rigid translation at large cell coordinates
changes energy and forces. Thread the requested dtype through convert
and pass it on the leftover build_dataloader path. The direct-graph
sibling already constructed positions at self.dtype.
@shaneraphel

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

This branch has not been deployed

No deployments
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