Dispersion - Githubissues

CederGroupHub / chgnet

Pretrained universal neural network potential for charge-informed atomistic modeling https://chgnet.lbl.gov

https://doi.org/10.1038/s42256-023-00716-3

Other

226 stars 61 forks source link

Dispersion #192

Closed ajhoffman1229 closed 1 week ago

ajhoffman1229 commented 2 weeks ago

Summary

Major changes:

tutorial: added a python notebook tutorial showing users how to add a dispersion correction to the main CHGNet model. Approving this would address #188 .

Checklist

This PR would not add any new features to the core code and only contains a notebook showing how to add dispersion to the CHGNet calculator through ASE. I installed pre-commit hooks and used them to format the code in the notebook.

[x] All existing tests pass.
[x] Tests have been added for any new features/fixes.
[x] Docstrings have been added in the Google docstring format.

ajhoffman1229 commented 2 weeks ago

I am not sure that DFT-D4 has a module available on pypi, you may need to use conda or mamba. Can you also install the python extension of DFT-D4? I think you will need conda install dftd4 dftd4-python -c conda-forge per the instructions here. I remember having a similar issue before when I installed the base dftd4 package without the dftd4-python component and tried to import DFT-D4. Without the python part, the import will fail.

Also, did you run pip install torch_dftd or pip install torch-dftd? I believe the latter is what is listed on pypi and I'm not sure the installation will work with an underscore.

janosh commented 2 weeks ago

I am not sure that DFT-D4 has a module available on pypi, you may need to use conda or mamba.

i see. not a fan of conda or mamba (strongly prefer uv). but that's not a PR blocker ofc, esp. for an optional dep. @BowenD-UCB you wanna try running the notebook? else happy to merge as is.

PS: pip install torch-dftd and pip install torch_dftd are equivalent.

ajhoffman1229 commented 2 weeks ago

I am not sure that DFT-D4 has a module available on pypi, you may need to use conda or mamba.

i see. not a fan of conda or mamba (strongly prefer uv). but that's not a blocker PR ofc, esp. for an optional dep. @BowenD-UCB you wanna try running the notebook? else happy to merge as is.

PS: pip install torch-dftd and pip install torch_dftd are equivalent.

I had not heard of uv before but I can understand the appeal, thank you for sharing! I like the idea of a rust-based environment manager.

BowenD-UCB commented 1 week ago

Hi @ajhoffman1229 and @janosh Thanks for the PR! Sorry for the delay on this. I tried to install these extra dependencies but encountered installation issue (package incompatibilities) with dftd4-python As these are optional deps I'll merge as it is now.