Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use get_cell().array #22

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

PythonFZ
Copy link

To fix the following error

UserWarning: Creating a tensor from a list of numpy.ndarrays is extremely slow. Please consider converting the list to a single numpy.ndarray with numpy.array() before converting to a tensor. (Triggered internally at ../torch/csrc/utils/tensor_new.cpp:261.)

One can access the get_cell().array attribute instead to get a np.ndarray instead of list[np.ndarray]

potential fix for #16

@corochann
Copy link
Member

Thank you for the PR!

Let me check my understanding. get_cell() always returns Cell class, and it is safe to assume that we can always access .array attribute to get 3x3 np.ndarray?

https://wiki.fysik.dtu.dk/ase/_modules/ase/atoms.html#Atoms.get_cell

@corochann
Copy link
Member

I commented on issue #16

@PythonFZ
Copy link
Author

PythonFZ commented Dec 8, 2023

From my understanding from https://wiki.fysik.dtu.dk/ase/_modules/ase/cell.html#Cell the conversion to numpy just does:

def __array__(self, dtype=float):
    if dtype != float:
        raise ValueError('Cannot convert cell to array of type {}'
                         .format(dtype))
    return self.array

such that using .array should not be an issue. It's a strange error in the first place tbh.

@corochann
Copy link
Member

If get_cell() always returns Cell it's ok, but I want to confirm get_cell() does not return numpy array or any other types? If such a case exists, .array is not accessible.

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