Skip to content

Conversation

@cw-tan
Copy link
Contributor

@cw-tan cw-tan commented Nov 20, 2025

Summary

Bug likely appeared because PBC is not a torch.Tensor instead of just a bool (#320). This failure mode only shows up when running on GPU, which explains why CI didn't catch it (presumably it's only run on CPU). I didn't check the other format conversion utility functions, but potentially some of the others might have a similar failure mode if they were not changed with the breaking PBC internal behavior change.

  File "/home/cw/harvard/matbench-discovery/models/nequip/torchsim_evals/test_nequip_discovery.py", line 172, in <module>
    relaxed_atoms_list = ts.io.state_to_atoms(final_state)
                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/cw/harvard/torch-sim/torch_sim/io.py", line 67, in state_to_atoms
    atoms = Atoms(
            ^^^^^^
  File "/home/cw/micromamba/envs/nequip/lib/python3.12/site-packages/ase/atoms.py", line 254, in __init__
    self.set_pbc(pbc)
  File "/home/cw/micromamba/envs/nequip/lib/python3.12/site-packages/ase/atoms.py", line 571, in set_pbc
    self.pbc = pbc
    ^^^^^^^^
  File "/home/cw/micromamba/envs/nequip/lib/python3.12/site-packages/ase/atoms.py", line 567, in pbc
    self._pbc[:] = pbc
    ~~~~~~~~~^^^
  File "/home/cw/micromamba/envs/nequip/lib/python3.12/site-packages/torch/_tensor.py", line 1213, in __array__
    return self.numpy().astype(dtype, copy=False)
           ^^^^^^^^^^^^
TypeError: can't convert cuda:0 device type tensor to numpy. Use Tensor.cpu() to copy the tensor to host memory first.

Checklist

Before a pull request can be merged, the following items must be checked:

  • Doc strings have been added in the Google docstring format.
  • Run ruff on your code.
  • Tests have been added for any new functionality or bug fixes.

Copy link
Collaborator

@curtischong curtischong left a comment

Choose a reason for hiding this comment

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

This looks fine. Thanks for catching this. We should probably run some of our tests on gpu

@curtischong curtischong merged commit 836ba32 into TorchSim:main Nov 22, 2025
139 checks passed
@cw-tan cw-tan deleted the fix-io-ase branch November 26, 2025 19:33
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.

2 participants