Skip to content

Pass oversubscribe status to MPI layer #8998

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 1 commit into from
Jun 2, 2021
Merged

Pass oversubscribe status to MPI layer #8998

merged 1 commit into from
Jun 2, 2021

Conversation

rhc54
Copy link
Contributor

@rhc54 rhc54 commented May 22, 2021

Update PMIx/PRRTE pointers to pass oversubscribe status
to child processes. Update OMPI to check for PMIx
attribute and set ompi_mpi_oversubscribe accordingly.
Move logic for setting yield_when_idle to a place after
the oversubscribe flag has been checked.

Signed-off-by: Ralph Castain rhc@pmix.org

@rhc54
Copy link
Contributor Author

rhc54 commented May 22, 2021

@jsquyres I believe this resolves your question on openpmix/openpmix#2192. PMIx and PRRTE are correctly passing the oversubscribe flag to OMPI. I have made an attempt at modifying the MPI layer logic to properly handle the flag - it seems to be working, but I defer to you to verify it. Feel free to modify it as necessary.

@dalcinl
Copy link
Contributor

dalcinl commented May 23, 2021

Looks like this had little effect on mpi4py testsuite running under GitHub Actions workers. The VMs have only 2 cores, the case Test mpi4py (np=3) takes forever to run. https://github.com/mpi4py/mpi4py-testing/runs/2650668709?check_suite_focus=true

At this point I think I should run these tests locally setting CPU cores offline to try to reproduce.

@jsquyres
Copy link
Member

jsquyres commented Jun 1, 2021

@rhc54 I confirm what @dalcinl was seeing: ompi_mpi_yield_when_idle was not getting set to true, even though PMIX was returning that we were in an oversubscribed situation.

I pushed a 2nd commit to this PR: I changed the logic of how ompi_mpi_yield_when_idle was set to true -- I don't think the original logic surrounding OPAL_THREAD_YIELD_WHEN_IDLE_DEFAULT was correct. Can you check?

If my 2nd commit is correct, let's squash it to the first commit and merge.

As of 1 June 2021, the PMIX and PRTE git submodule pointers on this PR are still newer than what are on master, so it's still ok to merge those updates as-is.

@rhc54
Copy link
Contributor Author

rhc54 commented Jun 1, 2021

@jsquyres It looks fine to me. I was trying to allow the user to override it if they chose to do so, though I can't imagine why someone would do that. I'll squash and re-push.

@jsquyres
Copy link
Member

jsquyres commented Jun 1, 2021

@jsquyres It looks fine to me. I was trying to allow the user to override it if they chose to do so, though I can't imagine why someone would do that. I'll squash and re-push.

The default value is passed up by the threads module. For pthreads, the default is false and the initial value is false, so it never used the value that came in from PMIx.

Since this PR removed the MCA param (which was internal, anyway), there's no exposed mechanism for the user to change this value. I think that's ok.

Update PMIx/PRRTE pointers to pass oversubscribe status
to child processes. Update OMPI to check for PMIx
attribute and set `ompi_mpi_oversubscribe` accordingly.
Move logic for setting yield_when_idle to a place after
the oversubscribe flag has been checked.

- change logic of setting ompi_mpi_yield_when_idle
- nit: change `ompi_mpi_oversubscribe` to `ompi_mpi_oversubscribed`
- add comment in ompi/runtime/params.h

Signed-off-by: Ralph Castain <rhc@pmix.org>
Signed-off-by: Jeff Squyres <jsquyres@cisco.com>
@jsquyres
Copy link
Member

jsquyres commented Jun 1, 2021

@dalcinl Can you try this PR again?

@dalcinl
Copy link
Contributor

dalcinl commented Jun 2, 2021

@jsquyres Yes! it seems it worked, see the logs. Look at the Test mpi4py (np=3) block, only 1 minute versus 19 minutes from the previous run before your changes.

@jsquyres jsquyres merged commit e26592e into open-mpi:master Jun 2, 2021
@jsquyres
Copy link
Member

jsquyres commented Jun 2, 2021

@rhc54 I'm unsure how to cherry pick this to the v5.0.x branch, because I think the Open MPI v5.0.x branch is tracking different PMIx / PRRTE branches than Open MPI master...?

@rhc54
Copy link
Contributor Author

rhc54 commented Jun 2, 2021

Add the MPI part to #9026

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.

3 participants