Skip to content

Detect that we have a mix of BE/LE in the system, provide a warning that OMPI doesn't currently support this environment, and error out #3828

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
merged 2 commits into from
Jul 7, 2017

Conversation

rhc54
Copy link
Contributor

@rhc54 rhc54 commented Jul 6, 2017

Signed-off-by: Ralph Castain [email protected]
(cherry picked from commit 2753f53)

…hat OMPI doesn't currently support this environment, and error out

Signed-off-by: Ralph Castain <[email protected]>
(cherry picked from commit 2753f53)
@rhc54 rhc54 added this to the v3.0.0 milestone Jul 6, 2017
@rhc54 rhc54 self-assigned this Jul 6, 2017
@rhc54 rhc54 requested a review from hppritcha July 6, 2017 22:41
@ggouaillardet
Copy link
Contributor

@rhc54 you need to cherry-pick 823382f

we do support mixing BE/LE, but only when configure'd with --enable-heterogeneous

@rhc54
Copy link
Contributor Author

rhc54 commented Jul 7, 2017

@ggouaillardet Are you sure that is true?? I looked but that PR you had created a long time ago was never committed, and so far as I can tell, we still don't work correctly in that mode.

@ggouaillardet
Copy link
Contributor

i will resume working on that PR.
as far as i am concerned, mixed BE/LE works pretty well in my environment (which might have a few not yet commited patches).
i am fine if v3.0.x does not support mixed BE/LE, but i will appreciate if master does.

@rhc54
Copy link
Contributor Author

rhc54 commented Jul 7, 2017

I'll leave this PR as-is until we see your PR completed. I'm a little disturbed about changing master without clear evidence that hetero is working again, but let's leave that in for now with the proviso that you get that PR completed and committed soon.

@hppritcha
Copy link
Member

hmm... well I now see something I'd like addressed if we're disabling BE/LE mixed support. Namely, we should at least for a while, remove the --enable-heterogeneous configure option if we know it doesn't work.

@rhc54
Copy link
Contributor Author

rhc54 commented Jul 7, 2017

Okay - easy enough to add. Will leave master alone, though, pending the PR from @ggouaillardet

@ggouaillardet
Copy link
Contributor

the PR is #2940

@rhc54
Copy link
Contributor Author

rhc54 commented Jul 7, 2017

@hppritcha I pushed the requested change

@bwbarrett
Copy link
Member

I'm not sure I like this patch... Let's push to master and then to v3.0.x. Yes, Giles is working on fixing heterogenous in master, but he can add a revert with the rest of his patch series. We need to get in the habit of not special casing all the branches.

@rhc54
Copy link
Contributor Author

rhc54 commented Jul 7, 2017

no problem - will do

Signed-off-by: Ralph Castain <[email protected]>
(cherry picked from commit 8e25733)
@rhc54
Copy link
Contributor Author

rhc54 commented Jul 7, 2017

Okay, I replaced the custom commit with the one to master: 8e25733

@bwbarrett bwbarrett merged commit fe41070 into open-mpi:v3.0.x Jul 7, 2017
@rhc54 rhc54 deleted the cmr30/bele branch July 7, 2017 21:39
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