Feat: Add DeepMD MLFF support #999
Conversation
|
@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. |
|
@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. |
|
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! |
tests/forcefields/test_md.py
Outdated
| @@ -143,8 +142,10 @@ def test_ml_ff_md_maker( | |||
| for step in task_doc.objects["trajectory"].frame_properties | |||
| ) | |||
|
|
|||
| with pytest.warns(FutureWarning): | |||
| name_to_maker[ff_name]() | |||
| # Skip the following test for DeepMD, since it doesn't have concrete implementations | |||
There was a problem hiding this comment.
Is it possible to implement this? Or do you see any hurdles?
There was a problem hiding this comment.
This part of the code checks whether the concrete Maker classes raise a FutureWarning, indicating they will be deprecated. As per our earlier discussions, I’ve removed the concrete implementations for DeepMD, so this class no longer exists. Therefore, I’ve addressed this by skipping the test for DeepMD.
There was a problem hiding this comment.
5MB is big for a test file. can we swap this out for a smaller checkpoint? or maybe auto-download the checkpoint on the fly like some of the other models?
There was a problem hiding this comment.
I have removed the original file and set it to download from GitHub when the test starts automatically.
tests/forcefields/test_jobs.py
Outdated
| # run the flow or job and ensure that it finished running successfully | ||
| responses = run_locally(job, ensure_success=True) | ||
|
|
||
| # validation the outputs of the job |
tests/forcefields/conftest.py
Outdated
| # Download DeepMD pretrained model from GitHub | ||
| file_url = "https://raw.github.com/sliutheorygroup/UniPero/main/model/graph.pb" | ||
| local_path = test_dir / "forcefields" / "deepmd" / "graph.pb" | ||
| response = requests.get(file_url, timeout=10) |
There was a problem hiding this comment.
atomate2 doesn't currently depend on requests so not sure it's installed. if it is, only as transitive dep. maybe better to use std lib urllib.request.urlretrieve
|
@janosh Hi, I found that the test can't pass because |
|
thanks for letting me know. could you replace tinyurl.com with one of the checkpoints from here? maybe |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #999 +/- ##
==========================================
- Coverage 76.09% 4.36% -71.74%
==========================================
Files 174 175 +1
Lines 12690 12742 +52
Branches 1892 1899 +7
==========================================
- Hits 9657 556 -9101
- Misses 2494 12152 +9658
+ Partials 539 34 -505
|
|
@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