Skip to content

hwloc: clarify --with-hwloc behavior #7252

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

Merged

Conversation

jsquyres
Copy link
Member

Clarify in README what --with-hwloc does in its different use cases.

Also, ensure that the behavior when specifying --with-hwloc is the
same as if that option is not specified at all. This is what we did
in Open MPI <= v3.x; looks like we inadvertantly caused --with-hwloc
to be synonymous with --with-hwloc=external in v4.0.0.

Signed-off-by: Jeff Squyres jsquyres@cisco.com

Thanks to @marmistrz on #6501 for the heads-up on this issue.

Clarify in README what --with-hwloc does in its different use cases.

Also, ensure that the behavior when specifying `--with-hwloc` is the
same as if that option is not specified at all.  This is what we did
in Open MPI <= v3.x; looks like we inadvertantly caused `--with-hwloc`
to be synonymous with `--with-hwloc=external` in v4.0.0.

Signed-off-by: Jeff Squyres <jsquyres@cisco.com>
@jsquyres
Copy link
Member Author

@rhc54 @gpaulsen @hppritcha You're all tagged on this PR for a review because it looks like we inadvertently caused --with-hwloc (with no value specified) to be equivalent to --with-hwloc=external in v4.0.0.

If we cherry-pick this back to v4.0.x, it means that --with-hwloc is equivalent to not specifying the option at all (i.e., first try external, and if that fails, fall back to internal).

I think that this is the behavior that we want.

The only question is if this breaks any installation scripts for >=v4.0.x -- i.e., are we breaking backwards compat. I don't think we are: if you specify --with-hwloc in >=v4.0.x, it currently fails if external can't be used. This change (fix!) relaxes that a bit: if external can't be used, it falls back to internal. It falls more in the spirit of "yes, do whatever you need to do to use hwloc" -- which IMNSHO is basically what --with-hwloc[=yes] should mean.

Thoughts?

Copy link

@marmistrz marmistrz left a comment

Choose a reason for hiding this comment

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

LGTM!

@hppritcha
Copy link
Member

I think this is a safe change.

@gpaulsen
Copy link
Member

I agree, I doubt this will break any scripts, is what we intended.

@jsquyres jsquyres merged commit 569d63c into open-mpi:master Dec 19, 2019
@jsquyres jsquyres deleted the pr/clarify-with-hwloc-functionality branch December 19, 2019 20:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants