Skip to content

Dispersion#192

Merged
bowen-bd merged 4 commits intoCederGroupHub:mainfrom
ajhoffman1229:dispersion
Aug 31, 2024
Merged

Dispersion#192
bowen-bd merged 4 commits intoCederGroupHub:mainfrom
ajhoffman1229:dispersion

Conversation

@ajhoffman1229
Copy link
Copy Markdown
Contributor

@ajhoffman1229 ajhoffman1229 commented Aug 23, 2024

Summary

Major changes:

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.

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

Copy link
Copy Markdown
Collaborator

@janosh janosh left a comment

Choose a reason for hiding this comment

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

after pip install torch_dftd dftd4, examples/dispersion.ipynb doesn't run for me. getting a kernel crash from the import cell:

Screenshot 2024-08-24 at 06 57 10

@janosh janosh added docs Improvements or additions to documentation ase Atomic simulation environment labels Aug 24, 2024
@ajhoffman1229
Copy link
Copy Markdown
Contributor Author

ajhoffman1229 commented Aug 27, 2024

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
Copy link
Copy Markdown
Collaborator

janosh commented Aug 27, 2024

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
Copy link
Copy Markdown
Contributor Author

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.

@bowen-bd
Copy link
Copy Markdown
Collaborator

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.

@bowen-bd bowen-bd merged commit 180ffea into CederGroupHub:main Aug 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ase Atomic simulation environment docs Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants