Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.04%. Comparing base (29de805) to head (4be1feb).
Report is 1 commits behind head on main.

@@           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.
📢 Have feedback on the report? Share it here.

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?

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 master, I can now properly revisit this PR.

There are really only two points to discuss:

1. For ONETEP, we can decide whether to: a) have a single ONETEP_CMD setting that is passed as-is to command in OnetepProfile (e.g. "mpirun -np 4 onetep.arch"), or b) have a ONETEP_CMD that is just the binary (e.g. "onetep.arch") and a separate ONETEP_PARALLEL_CMD that is the parallelization string that comes before the binary (e.g. "mpirun -np 4"). I am inclined to do the former since it is inherently more flexible.
2. For Espresso, it's a bit tricky because we can't just use a single ESPRESSO_CMD passed to command in EspressoProfile since there is some automated logic for determining the binary. If we want to add commands before and after the binary per your request (which is indeed supported by ASE), we'll have to come up with a clean way to do that via the settings. It's mostly just a matter of preference.

@Andrew-S-Rosen

1. Former is nicer I agree
2. I am not sure I will let you decide on that one. Being able to do something like ESPRESSO_PARALLEL_CMD = "blablabla {binary} blablabla" would be nice?

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.

1. For ONETEP, I went with the single ONETEP_CMD: str option, which is essentially identical to the ASE command in the profile.
2. For Espresso, I went with an ESPRESSO_PARALLEL_CMD: str | tuple[str, str] approach for maximum flexibility. With the tuple[str, str], the 0th index is the string that comes before the binary, and the 1st index is the string that comes afterwards. For convenience, the user can also supply a single str, in which case it will be transformed internally to str, ".

I'll merge this in for now to unbreak the tests since parallel_info is gone in ASE, but feel free to propose changes as you see fit.