Skip to content
This repository has been archived by the owner on Apr 24, 2024. It is now read-only.

Should ASE be a core dependency? #29

Open
jwa7 opened this issue Mar 3, 2023 · 2 comments
Open

Should ASE be a core dependency? #29

jwa7 opened this issue Mar 3, 2023 · 2 comments

Comments

@jwa7
Copy link
Member

jwa7 commented Mar 3, 2023

In equisolve/utils/convert.py, ase is used in the ase_to_tensormap() fxn. If not already installed, a ModuleNotFoundError is raised. Should ASE be a core dependency?

@Luthaf
Copy link
Collaborator

Luthaf commented Mar 3, 2023

I don't think it should be, equisolve does not need ase to work. But the ase import should be guarded when defining the ase_to_tensormap function (if ase is not available, this function will never be used anyway) to make sure one can use equisolve without ase

@agoscinski
Copy link
Collaborator

We just do it like in chemiscope and make a HAS_ASE flag, as we also will do it with for torch

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants