Draft
Conversation
Merged
Member
Author
|
I don't have the time to carefully check the MPI dependency now. I'm still a bit afraid here that we might oversee something, if we merge this PR, but only having MPI as an optional dependency would still be nice. I would suggest to keep this issue. If somebody finds time to work on it or the MPI dependency really becomes a problem (because one cannot easily satisfy it on a special system, for example), we can start looking into this again. |
mosayebshams
added a commit
to gymprecice/gymprecice
that referenced
this pull request
Mar 3, 2023
precice seems to be at the root of our issue with not being able to fork mpirun within gymprecice. The main reason is that, within the precice Python-binding, developers have added line "from mpi4py import MPI" to deal with a dependency compile error (as a workaround), which is still an open issue: The issue: precice/python-bindings#8 (comment) The PR which has not been merged into the release branch: precice/python-bindings#101 and precice/precice#299 Based on https://stackoverflow.com/questions/64581965/run-mpi-program-using-subprocess-popen, the added line, "from mpi4py import MPI", makes the python script to be run in singleton mode, and therefore: sunprocess.run(["mpirun"], ...) fails sunprocess.Popen(["mpirun"], ...) is zombified. This fix is a workaround to remove MPI from the parent env and provide the "cleaned" env to subprocess rather than letting the subprocess inherit the parent env as it is. The fix is inspired by the discussion in: https://stackoverflow.com/questions/30384649/python-with-embedded-call-to-mpirun
11 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR removes mpi4py (again) as a dependency.
There is some history and some other issues involved. I just added a test in #100 to be able to reproduce possible failures. Before merging this PR I would like to investigate the setup described in #8 (comment) again and make sure that the error is really gone (ideally also find out why it is now gone).