Open MoseyQAQ opened 1 month ago
@MoseyQAQ Thanks for your contribution.
As you might have seen, we want to deprecate the concrete implementations of the force fields in the future and just provide a general interface. I am not sure if it makes sense to add this new implementation just now and directly deprecate it in a few months.
@utf @janosh Opinions?
@JaGeo Hi, thanks for your comment! I opted for concrete implementations because the atomate2 workflow for calculating phonons tutorial uses concrete implementations. I aim to maintain consistency with the established practices in these resources to ensure a seamless user experience.
@utf Opinions?
@MoseyQAQ Yes, sure, it does. However, we decided to change this in the future. You can even see this from the code that you implemented.
Thanks for this @MoseyQAQ. I agree with @JaGeo, there doesn't seem much point adding something already with a deprecation notice. I would remove the concrete implementations but leave the rest of your code. DeepMD is great to have in atomate2.
In the future, we should think about convenience functions to create entire flows with a specific forcefield. E.g., something like:
from atomate2.forcefields.flows import ElasticMaker
maker = ElasticMaker.from_forcefield("DeepMD")
@utf Thank you!
@utf @JaGeo , Hi! I’m currently working on removing the old implementations as suggested and addressing the issue preventing the code from passing the unit tests (which might be related to the dependency version). I’ll update as I make progress.
@utf @JaGeo Hi! This PR is ready for review and could be merged.
@janosh Hi, I found that the test can't pass because MACE
is unable to download the foundation model from tinyurl.com
. In the latest release of MACE
, the original tinyurl.com
link has been replaced with GitHub
. The previous tinyurl.com
URL returns a 403
error, which prevents the model from being downloaded properly.
thanks for letting me know. could you replace tinyurl.com with one of the checkpoints from here? maybe 2023-12-10-mace-128-L0_epoch-199.model
Attention: Patch coverage is 0%
with 6 lines
in your changes missing coverage. Please review.
Project coverage is 4.36%. Comparing base (
ec1b598
) to head (b012a93
). Report is 5 commits behind head on main.
Files with missing lines | Patch % | Lines |
---|---|---|
src/atomate2/forcefields/utils.py | 0.00% | 5 Missing :warning: |
src/atomate2/forcefields/__init__.py | 0.00% | 1 Missing :warning: |
@utf : I added that kind of convenience function for the forcefield EOS flows but didn't for more complex ones - happy to do that in a separate PR
Thanks @esoteric-ephemera and @MoseyQAQ. Is this ready for final review?
@utf Yes. @esoteric-ephemera Thank you!
Summary
I have integrated atomate2 with the DeepMD MLFF, enabling efficient high-throughput calculations using this powerful tool.
Checklist