Skip to content

Conversation

@joezuntz
Copy link
Contributor

@joezuntz joezuntz commented Aug 6, 2021

The current version of the code always uses MPI_COMM_WORLD as the communicator and calls sys.exit on some processes when the client process exits python.

This change makes it possible to use dask-mpi for part of an MPI process but then return to the original MPI workflow once it is complete. It requires the user to manually call send_close_signal from the client process, and to use a returned value indicating whether the process is the client or not.

@joezuntz
Copy link
Contributor Author

joezuntz commented Aug 6, 2021

The test failures shown seem to be part of the original code that is not affected by the changes I've made, and are associated with a timing test. I guess this might be associated with github actions having slowed down since these were written?

@kmpaul
Copy link
Collaborator

kmpaul commented Aug 25, 2021

Some of the errors seen in these tests are related to the bug mentioned in #74.

@kmpaul
Copy link
Collaborator

kmpaul commented Aug 30, 2021

@joezuntz: Can you pull recent updates from the main branch and see what tests pass/fail?

@joezuntz
Copy link
Contributor Author

I just did this but it seems to show only a single change, to cli.py - is that what you expected?

@joezuntz
Copy link
Contributor Author

Also it says it's "awaiting approval" now: "First-time contributors need a maintainer to approve running workflows"

@kmpaul
Copy link
Collaborator

kmpaul commented Aug 30, 2021

@joezuntz: Yay! Tests pass, and the failures had nothing to do with your code.

@kmpaul
Copy link
Collaborator

kmpaul commented Aug 30, 2021

@joezuntz: Is this ready to merge, then?

@joezuntz
Copy link
Contributor Author

Yep, I think good to go - thanks!

@kmpaul
Copy link
Collaborator

kmpaul commented Aug 30, 2021

Thanks @joezuntz!

@kmpaul kmpaul merged commit abc3f9e into dask:main Aug 30, 2021
@kmpaul kmpaul mentioned this pull request Aug 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants