-
Notifications
You must be signed in to change notification settings - Fork 53
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
Add support for new ASE profile format #2061
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2061 +/- ##
=======================================
Coverage 99.04% 99.04%
=======================================
Files 81 81
Lines 3357 3364 +7
=======================================
+ Hits 3325 3332 +7
Misses 32 32 ☔ View full report in Codecov by Sentry. |
…ors/quacc into Andrew-S-Rosen-patch-1
…ors/quacc into Andrew-S-Rosen-patch-1
I had a quick look and I am not sure if this allows for command suffix as well? In espresso something like "mpirun -v -np 10 pw.x -nk 2 -nt 8 -ndiag 36 -in pw.in" is possible, but not supported currently, will the new mechanism in https://gitlab.com/ase/ase/-/merge_requests/3343/diffs#1107270596d43f253c632b6ffb2ce9c5e75460cb take care of that? |
This should be possible in ASE. It was agreed upon among the maintainers. Whether it is functional yet or is different. The MR still needs testing. In terms of this PR, it's also still a WIP but I'll return to this when the ASE MR is merged. 👍 |
@tomdemeyere: With the ASE changes merged into There are really only two points to discuss:
|
It would also be nice to make sure that this is compatible to build the command automatically with Parsl as per #2076 but that complicates things a little bit more. |
@tomdemeyere: Thanks for your input! This should be taken care of now.
I'll merge this in for now to unbreak the tests since |
Summary of Changes
This PR seeks to streamline the handling of parallelization information across codes in quacc, namely to update the behavior for the Espresso and ONETEP recipes that are based on GenericFileIOCalculator. This PR closes #1532 and is reliant on the merge of https://gitlab.com/ase/ase/-/merge_requests/3343.
In short, handling of
parallel_info
is avoided. Instead, users provide quacc settings that will automatically apply in the instantiated Profile object'scommand
keyword argument.For ONETEP, that means there is now a single
ONETEP_CMD
that is astr
.For Espresso, that means
parallel_info
is removed from each recipe and in its place there is anESPRESSO_PARALLEL_CMD
setting that takes astr
ortuple[str]
Then behavior is the same as every other recipe.
Checklist
main
.Notes