-
Notifications
You must be signed in to change notification settings - Fork 900
Re-enable runtime-enabled configure
CLI params (to pass to PRRTE)
#7486
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
Comments
I think this needs to be a blocker for v5. Otherwise, I don't know how you compile in support for things like Slurm and LSF into PRRTE from the OMPI configure. This fell off my radar, but I'll try to get to it this week. |
Brian said to wait until the branch he posts later today. |
Ref #8384 |
I got the all-clear from Brian that nothing more is expected. I have a branch in progress for this that I hope to have up this week. |
Problem
Solution
Left discuss/complete
|
I created a draft PR in #8409 with the design above. |
I think we should kill the PR. I think Jeff's original issue is incorrect. THe right way to handle this in Autoconf is to use --help=recursive. All those options still have meaning to recursive packages, and there is an existing, Autoconf-approved way of seeing them today, without all the extra code. I also really want to understand the renaming idea; that seems very bad, and we intentionally didn't do that when the third party patches were written, because they only lead to madness of package selection differing between 3rd party packages (and OMPI). |
I wouldn't do the renaming - I agree with that concern. The problem is that the world out there (a) doesn't know or care that OMPI suddenly added submodules, and therefore (b) doesn't know that they now have to do something different than normal for discovering the configure options. We had questions coming in (some actually from Amazon!) about OMPI suddenly removing RTE options and concerns that we had arbitrarily dropped support for various HPC environments. It's easy to understand why people would think that - if you do what you have done for nearly 20 years, those options are all gone. If you created any automation for configuring OMPI (and some people, like the labs and packagers, have done so), then that automation is suddenly failing due to lost configure options. So I don't know the right answer here. I understand your objection, but that may not be what users will accept. Regardless, I don't really care as this is an OMPI issue - my PR was solely a stopgap to respond to the concerns coming (from Amazon as well!) about lost options and discontinued support. |
|
To be clear, I said that I was not working on anything in the 3rd party code right now, not that I thought this was a good idea. I thought I made it pretty clear I had no idea what was going on, other than seeing my name go by in a proposal. I'm one person, who raised objections to the PR because it's not how Autoconf expects packages to work. Maybe we still should do the argument forwarding. I tend to disagree, but we should have that discussion, not throw up our arms. I do have very strong feelings about argument renaming. While I understand your points, I think 1) that we need to be shooting for less third party packages over time (within 1-2 years, we should be able to remove libevent, for example), 2) that we do need to think about feature propagation carefully, outside of argument naming, and 3) that the risk of someone building with conflicting SLURM arguments because of our renaming far outweighs the small potential benefits of the feature. We do need to look at your question about ./configure --help=recursive. I don't know off the top of my head. I do know that that is how Autoconf expects this situation to be handled. |
There are a small number of people familiar enough with the build system to do this work, and thus have opinions about how it should function. All I ask is that you take care in the future either directly or indirectly green lighting a ticket like this, especially if there is more context to add (i.e., your prior conversation with Ralph about this topic) so folks don't waste time. It was not clear to me that you had "no idea what was going on". To aid in the discussion, there are two technical points being discussed here:
Ultimately I don't care if the community doesn't want to take the direction seen in PR #8409 - the best solution should be taken no matter what. What the community must do is outline explicitly what they want to do here before someone else wastes effort on a solution. |
Per Teleconf Feb. 2, 2021:
|
Per Teleconf Feb. 9, 2021:
|
I investigated the duplicate configure options. If a configure option is declared more than once then it appears more than once in the For example (top from OMPI's config and bottom from OpenPMIx's config):
I will need to work on some deduplication. |
@jjhursey I was curious why this wasn't a problem with all the places we call OMPI_CHECK_PACKAGE for multiple components. You'd expect if we don't dedupe to see three --with-ucx help strings, for example. Reading the autoconf m4 code, the dedupe is only if the entire help string is identical. In this case, Autoconf didn't dedupe because the description is different ("not for general MPI users" vs. "not for general PMIx users"). That probably makes your job harder, but that's what Autoconf does... |
Yeah, I noticed it wasn't 100% of the time that it duplicated. For example PRTE's
Which matched OMPI's (something we should fix in PRTE), but it did not show up twice. Only the OMPI and OpenPMIx versions. I did not deep dive into why though. Thanks for figuring that out. Checking the help strings does make the parsing a bit trickier, but possible if needed. However, if we have multiple options marked with |
Yes, you're right, just deduping on the AC_ARG_ENABLE first option seems like the right thing to do here, particularly if that's easiest for you. I don't think the corner cases of descriptions really matters. Mostly, your statement about deduping was an itch that I had to scratch :). |
The master README currently still describes the following
configure
CLI params:With the exception of
--with-pmi
(which is already handled) and--enable-orte-static-ports
(see below), they others don't seem to exist in OMPI's./configure --help
output any more. Some plumbing needs to be put in to support this kind of functionality in some manner (e.g., just pass them directly through to PRRTE'sconfigure
, or ...something else...).I don't know if PRRTE supports static ports like ORTE did, but if so, that one should probably be plumed through, too. If not,
--enable-orte-static-ports
should probably be added toompi_deleted_options.m4
.In Slack discussions, @jjhursey volunteered to look at this.
The text was updated successfully, but these errors were encountered: