Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor and small updates to OpenMM and OpenFF task documents #1087

Merged
merged 18 commits into from
Sep 9, 2024

Conversation

orionarcher
Copy link
Contributor

@orionarcher orionarcher commented Sep 5, 2024

This PR implements changes to the core OpenMM and OpenFF task docs to support changes to the atomate2 workflows downstream. It is intended to split off the functionality changes in emmet-core implemented in PR #1010, which was becoming too large and burdensome.

  • changes the directory structure
  • change the names and type signatures of some task_doc elements

I cleaned up the commit history so that it's easier to inspect. @tsmathis.

@orionarcher orionarcher changed the title Md core Refactor and small updates to OpenMM and OpenFF task documents Sep 5, 2024
@tsmathis
Copy link
Collaborator

tsmathis commented Sep 5, 2024

nice, thanks @orionarcher

just to confirm this one will get your downstream work in atomate2 moving?

…related dependencies and allow installation from conda-forge.
@codecov-commenter
Copy link

codecov-commenter commented Sep 5, 2024

Codecov Report

Attention: Patch coverage is 66.66667% with 19 lines in your changes missing coverage. Please review.

Project coverage is 89.98%. Comparing base (3b414c6) to head (a1ede7c).

Files with missing lines Patch % Lines
emmet-core/emmet/core/openmm/tasks.py 71.05% 11 Missing ⚠️
emmet-core/emmet/core/openff/tasks.py 52.94% 8 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1087      +/-   ##
==========================================
- Coverage   90.03%   89.98%   -0.05%     
==========================================
  Files         147      147              
  Lines       13460    13483      +23     
==========================================
+ Hits        12119    12133      +14     
- Misses       1341     1350       +9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@orionarcher
Copy link
Contributor Author

Yep, just tested this with the atomate2 code and it's all working locally.

emmet-core/setup.py Outdated Show resolved Hide resolved
emmet-core/setup.py Outdated Show resolved Hide resolved
Remove SolvationDoc and updated documentation of CalculationsDoc

Fix dependencies in setup.py

Remove warnings
@tsmathis
Copy link
Collaborator

tsmathis commented Sep 5, 2024

any further changes you want in @orionarcher?

@tsmathis
Copy link
Collaborator

tsmathis commented Sep 5, 2024

@tschaume, some updates to reqs files were needed to get this through, namely a pin on dgl for the ml extras. But this is likely since I was compiling the reqs locally and dgl was upgrading.

But it does seem like dgl does not have a ubuntu-compatible version >2.1 on pypi (failed actions here: failed run

so a re-run of the deps upgrade will be needed I think after this is in

@tschaume
Copy link
Member

tschaume commented Sep 5, 2024

@tsmathis ok, thanks for the heads-up! I can rerun the dependencies and take a closer look after this is merged.

@orionarcher
Copy link
Contributor Author

orionarcher commented Sep 6, 2024

This is good to merge. If there are any other changes/updates I can include them in a future smaller PR.

Scratch that, found some weird interactions with jobflow-remote that I am working on debugging.

@tsmathis
Copy link
Collaborator

tsmathis commented Sep 6, 2024

This is good to merge. If there are any other changes/updates I can include them in a future smaller PR.

Scratch that, found some weird interactions with jobflow-remote that I am working on debugging.

All good, either ping us here or on slack when you're ready

@orionarcher
Copy link
Contributor Author

@tsmathis, fixed. This should be good to merge when tests pass.

@tsmathis
Copy link
Collaborator

tsmathis commented Sep 9, 2024

👍

@tsmathis tsmathis merged commit 3b1060f into materialsproject:main Sep 9, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants