mpi4py: run the spawn and dynamic process tests #12421
Conversation
@jsquyres Are you sure? Isn't better to leave the release branch as it now, such that any mpi4py failures would signal an new, unknown regression in the release branch? |
jsquyres
force-pushed
the
pr/mpi4py-run-the-spawn-tests
branch
3 times, most recently
from
58710e4 to
0fbbebd
Compare
|
I thought about this overnight. I changed up this PR a bit, and split the Github Action into 4 parts:
Keeping the spawn and dynamics tests separate are useful because those are different failures. So let's run them separately. This gives us a clear green checkmark if all the mpi4py tests -- excluding spawn and dynamic tests -- pass. Then it separately runs the spawn and dynamic tests, which we currently expect to fail. Looks like I don't quite have the syntax correct yet for this in the github action -- let me go fool with this over in my fork and not spam everyone here with a bunch of force pushes until I get this right. |
Feel free to ping me about any doubts you have about GHA, I've been using the thing for quite a while. |
jsquyres
force-pushed
the
pr/mpi4py-run-the-spawn-tests
branch
2 times, most recently
from
aa2129d to
5d05c49
Compare
|
@bosilca @hppritcha I revamped the github action quite a bit since you approved it. Please re-review. Here's what the output looks like now in the CI on the PR:
And here's what it looks like if you click through to the github action result itself:
|
jsquyres
force-pushed
the
pr/mpi4py-run-the-spawn-tests
branch
from
5d05c49 to
e96f5f7
Compare
|
General consensus on the call today:
That way, the tests are still there and able to be run, but a developer has to do something to run them. |
jsquyres
force-pushed
the
pr/mpi4py-run-the-spawn-tests
branch
from
e96f5f7 to
635a663
Compare
jsquyres
force-pushed
the
pr/mpi4py-run-the-spawn-tests
branch
from
635a663 to
027009a
Compare
jsquyres
force-pushed
the
pr/mpi4py-run-the-spawn-tests
branch
from
027009a to
e255ffc
Compare
Split the mpi4py Github Action into 4 parts: 1. build: do everything to build, configure, and install Open MPI and mpi4py 2. run: run all the mpi4py tests with its defaults. As of March 2024, this disables the spawn and dynamic tests, which means that the entire block of tests should pass. 3. run_spawn: run all the mpi4py tests, including the spawn tests, but only if the "mpi4py" label is set on the PR. As of March 2024, we know some of these tests are failing. 4. run_dynamic: run all the mpi4py tests, including the dynamic tests, but only if the "mpi4py" label is set on the PR. As of March 2024, we know some of these tests are failing. The spawn and dynamic failures are different, so we split them up and run them separately. For steps 2, 3, and 4, we utilize a reusable Github workflow so that we don't have to duplicate the code. Signed-off-by: Jeff Squyres <jeff@squyres.com>
jsquyres
force-pushed
the
pr/mpi4py-run-the-spawn-tests
branch
from
e255ffc to
deb4bda
Compare
|
Ok, I think I got it now. Here's what a run looks like if the
And here's what a run looks like if the
|
| # This parameter is required, so send a meaningless | ||
| # environment variable name that will not affect the tests at | ||
| # all (i.e., the tests will be run with default values). | ||
| env_name: MAKE_TODAY_AN_OMPI_DAY |
They're currently disabled by default upstream in mpi4py because Open MPI is failing these tests. Explicitly enable them here in Open MPI's github action for mpi4py so that we have to fix them.
See mpi4py/mpi4py#479 for some details.
FYI @bosilca @dalcinl @hppritcha
Once this merges, we'll bring this over to v5.0.x.