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

Support MPI executables (on a single process) #47

Merged
merged 3 commits into from
Nov 21, 2022

Conversation

ltalirz
Copy link
Member

@ltalirz ltalirz commented Nov 19, 2020

This builds on #45 and makes it possible to run executables through mpirun, but on a single-process only(!).

Previously, when the mock executable would detect that an input is not present in the cache, it would

  • replace its name in the submit script with the fallback executable
  • run the submit script (again) using bash

This approach obviously does not work in conjunction with MPI, since it ends up calling mpirun twice: once for running the mock executable, and once again inside the mock executable when it calls the submit script again.

This PR changes the mock executable to simply call the fallback executable directly, using the command line arguments that were provided.
Environment variables are forwarded to the subprocess automatically using subprocess.call.

This works for running with -np 1, but it still does not solve running with more than one process (one should probably add a check).

Previously, the mock executable would
 * replace its name in the submit script with the fallback executable
 * run the submit script using bash

This approach obviously does not work in conjunction with MPI, since it
ends up calling `mpirun` inside `mpirun.

This PR changes the mock executable to simply call the fallback
executable directly, using the command line arguments that were
provided.
Environment variables are forwarded to the subprocess automatically
using `subprocess.call`.
@ltalirz
Copy link
Member Author

ltalirz commented Nov 21, 2022

Reviving this pull request, as it is used in aiida-lsmo required by @mpougin

@greschd do you see any obvious issues with this approach?

greschd
greschd previously approved these changes Nov 21, 2022
aiida_testing/mock_code/_cli.py Outdated Show resolved Hide resolved
@greschd
Copy link
Member

greschd commented Nov 21, 2022

LGTM, apart from the suggestion above (remove outdated comment).

Co-authored-by: Dominik Gresch <greschd@users.noreply.github.com>
@ltalirz
Copy link
Member Author

ltalirz commented Nov 21, 2022

Thanks for the quick review @greschd, much appreciated!

@ltalirz ltalirz merged commit c41027f into aiidateam:develop Nov 21, 2022
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.

2 participants