Skip to content

Conversation

awlauria
Copy link
Contributor

@awlauria awlauria commented Feb 3, 2022

Fixes a bug where out-of-the-box ompi builds with an
external prrte will fail to find prterun.

Signed-off-by: Austen Lauria [email protected]

Fixes a bug where out-of-the-box ompi builds with an
external prrte will fail to find prterun.

Signed-off-by: Austen Lauria <[email protected]>
Copy link
Member

@bwbarrett bwbarrett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand this patch. There is not supposed to be a prterun in the OMPI install tree. If you're using an external PRRTE, the expectation is that PRRTE's install_prefix/bin is in your path.

@awlauria
Copy link
Contributor Author

awlauria commented Feb 3, 2022

If adding a symlink is a problem an easy alternative would be to just have mpirun resolve it via PRTE_PATH.

Of course it could be solved by using --prefix ore adding it to the PATH, but I'm not sure why we couldn't make this a bit easier for users.

@rhc54
Copy link
Contributor

rhc54 commented Feb 3, 2022

Guess it all depends on how much you want to expose OMPI internals to the user. Been getting some hits back where someone else installs OMPI, and then a user (who doesn't know or care what PRRTE is) gets a "cannot find the prterun executable" when trying to execute mpirun. The immediate response is "what the heck is prterun?", upon which you have to laboriously explain what PRRTE is and how OMPI uses it, and then tell them they have to modify their paths.

The goal of this patch was to remove that ambiguity by ensuring mpirun can find prterun, thereby saving a lot of repetitive explanations.

@bwbarrett
Copy link
Member

I'm all for making sure that happens, but we have an mpirun executable. Having it properly do a path search seems like a much better option than adding a symlink so that there are now two prteruns in your path.

@awlauria
Copy link
Contributor Author

awlauria commented Feb 3, 2022

Sounds good - opened #9963 to implement that.

@awlauria awlauria closed this Feb 3, 2022
@awlauria awlauria deleted the fix_mpirun_external branch February 4, 2022 21:08
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.

3 participants