-
Notifications
You must be signed in to change notification settings - Fork 49
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
bugfix: Default MPI Command #508
Conversation
…in node so default MPI command changed to ''. This might be useful for short runs. - Script reports that it adds 'xterm -e' to MPI command if not already present and debug mode turned on. User might be using a different method for debugging MPI so this info is useful to report
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no need for type conversions here, since the empty string is evaluated as False
in logical checks anyway.
@scrasmussen How has this been tested? Just the CI tests below, or did you try Hera/Derecho/something else with login node and compute node? |
FYI, on Hera login node, using "vanilla" stdout: "" |
@scrasmussen Same error with compute node |
@grantfirl I'm not surprised by these results, since these changes make the run command blank by default. If you include |
Yes. Right now, I'm not in favor of making these changes (i.e. merging this PR) since I think that more people use Hera than Derecho for the SCM. Why can't we just change the SCM UG to mention that one needs to know how to run with MPI on their machine and tell them what the default is? Another way to go would be to set an environment variable in modulefiles for the MPI command to use and then reference that from the run script. IMO, I think that the code is fine to release without these changes and just explain in the documentation. |
I actually like this idea best, but I agree it's probably a bit much to get in for this release. We do need some kind of fix because the current code does not allow you to not specify @scrasmussen What do you think? |
New default to work on Hera. Derecho will require --mpi_command='' Co-authored-by: Michael Kavulich <[email protected]>
@grantfirl @mkavulich Thanks for testing this on Hera! That's frustrating that the two different MPI implementations/versions deal with the linking differently. It seems Derecho's MPI will not call any MPI functions when being run in serial, so it doesn't need the I committed Mike's recommended change of using |
@grantfirl @mkavulich I've created a new branch with documentation changes for |
DESCRIPTION OF ISSUES AND CHANGES:
mpiexec
ormpirun
command will not run, even if a single process is requested. The default MPI command wasmpirun -np 1
, default MPI command now changed to ''.xterm -e
could potentially cause issues. It probably won't be an issue, so script only reports that it adds 'xterm -e' to MPI command if debug mode turned on andxterm
was not passed by the user.