Skip to content

README.md: make the heterogeneous support more clear #9811

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

@jsquyres jsquyres commented Jan 2, 2022

Remove ambiguous warning language (it's not clear if the "THIS
FUNCTIONALITY..." warning applies to the option above or below the
warning) and make it clear exactly the heterogeneous option supports
and does not support.

Signed-off-by: Jeff Squyres [email protected]

@jsquyres
Copy link
Member Author

jsquyres commented Jan 2, 2022

Hey @bosilca

  1. Is the heterogeneous functionality still broken on master/v5.0.x? I couldn't find an open ticket on it, and my memory isn't good enough to remember the current state of heterogeneous support.

    I know we had a bunch of partial datatype fixes in the past 12 months or so, but I don't think that those were directly related to heterogeneity -- although Reenable heterogeneous support. #8735, Fix compile failure with enable-heterogeneous. #9699, and v5.0.x: Fix compile failure with enable-heterogeneous. #9700 seem to imply that heterogeneous functionality may actually be working, and we should remove these warnings...? If heterogeneous support is working on master / v5.0.x, this PR can be changed to remove the warning.

  2. Is heterogeneous functionality still broken on v4.0.x/v4.1.x? Per Compile fail with --enable-heterogeneous #9697 (comment), we had the "don't use!" warning accidentally in the wrong section. But now I wonder if heterogeneity is actually broken at all on v4.0.x / v4.1.x... Do you remember?

    Currently, v4.0.x: README: update --enable-heterogeneous warning #9809 and v4.1.x: README: update --enable-heterogeneous warning #9810 are open to fix the README's on v4.0.x / v4.1.x -- they could be changed to remove the warnings if heterogeneous support is actually working on those branches.

This raises the larger question: if heterogeneous support is known to be broken, why don't we have configure fail if --enable-heterogeneous is specified? Or even just remove that option altogether? (it could be removed temporarily if someone plans to fix the DDT heterogeneity support someday)

@bosilca
Copy link
Member

bosilca commented Jan 2, 2022

I don't think the heterogeneous support is entirely broken, it depends on the source and target architectures. If I remember well, the conversion of floating point numbers between architectures with a different number of bits in the exponent/mantissa might not be fully supported.

@jsquyres
Copy link
Member Author

jsquyres commented Jan 2, 2022

Is that the case on all of v4.0.x, v4.1.x, master, and v5.0.x?

Regardless, in the branch(es) where that is the case, what do you suggest we do -- document something like "this may work, but it may not work when using on systems with differing numbers of bits in the exponent/mantissa blah blah blah"?

@jsquyres
Copy link
Member Author

jsquyres commented Feb 1, 2022

@bosilca and I had an offline discussion about this. Yes, the conclusion is that heterogeneous support should work, but it is really focused on endian swapping. If type sizes are different -- to include differing numbers of bits in the mantissa or exponent -- problems can/will arise. I will re-word the explanation to be more explicit.

@jsquyres jsquyres force-pushed the pr/heterogeneous-functionality-is-broken branch from be7efee to f9aa56f Compare February 1, 2022 15:40
@jsquyres jsquyres changed the title README.md: make the heterogeneous warning more clear README.md: make the heterogeneous support more clear Feb 1, 2022
@jsquyres jsquyres force-pushed the pr/heterogeneous-functionality-is-broken branch 2 times, most recently from 3c18d06 to 0098615 Compare February 3, 2022 20:55
Remove ambiguous warning language (it's not clear if the "THIS
FUNCTIONALITY..." warning applies to the option above or below the
warning) and make it clear exactly the heterogeneous option supports
and does not support.

Signed-off-by: Jeff Squyres <[email protected]>
@jsquyres jsquyres force-pushed the pr/heterogeneous-functionality-is-broken branch from 0098615 to 0b0bae0 Compare February 3, 2022 20:56
@open-mpi open-mpi deleted a comment from ibm-ompi Feb 3, 2022
@jsquyres
Copy link
Member Author

jsquyres commented Feb 3, 2022

bot:ompi:retest

@jsquyres jsquyres merged commit 43524c4 into open-mpi:master Feb 4, 2022
@jsquyres jsquyres deleted the pr/heterogeneous-functionality-is-broken branch February 4, 2022 13:13
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.

2 participants