-
Notifications
You must be signed in to change notification settings - Fork 900
Extract 3rd party configure options for OMPI configure #8409
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
Conversation
f67541a
to
bbb5ddf
Compare
Details on the design are here Left to do:
|
The revert should definitely be committed, as that patch should really never have been added. I obviously think the scripts to replicate the behavior of I'm curious the use case for having a different location of SLURM (or similar) for Open MPI and PRRTE. I talked to Ralph when doing the initial work, and we could not come up with one that seemed useful enough for that code (and the complexity of users accidentally specifying a package enablement to prrte but not pmix, for example. So I'd really rather we not add that functionality without a bunch more discussion. |
Let's continue the discussion on the original ticket to reach a resolution: Issue #7486 |
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.
I only reviewed the first two patches, since the third (the renaming patch) won't be needed.
This reverts commit 540b2dc. Signed-off-by: Joshua Hursey <[email protected]>
1f6181b
to
f12d819
Compare
Left to do:
|
* Extracts configure arguments from 3rd party packages * Takes care to de-duplicate options already provided by OMPI - So there is a preference for the OMPI configure option with its help string. Signed-off-by: Joshua Hursey <[email protected]>
This PR is now ready for review. I believe that it has addressed all of the requirements that the community has defined for this functionality from Issue #7486. |
Does anyone else want to review before I merge? Otherwise, I'll bring this in tomorrow. |
configure
CLI params (to pass to PRRTE) #7486