-
Notifications
You must be signed in to change notification settings - Fork 199
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
Move MPI behavior from HTEX to MPIExecutor #3582
Conversation
65abe9b
to
3628427
Compare
provider=SlurmProvider(launcher=launcher), | ||
) | ||
]) | ||
|
||
|
||
@pytest.mark.local | ||
def test_correct_launcher_with_mpi_mode(): | ||
def test_bad_mpi_launcher(): | ||
"""ValueError if an unsupported mpi_launcher is specified""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this test looks for a type error not a value error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's meant to be a test for a value error though, right?
ff92e88
to
6aef5dd
Compare
test-requirements.txt
Outdated
@@ -25,4 +25,3 @@ Sphinx==4.5.0 | |||
twine | |||
wheel | |||
isort | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are you trolling me
* Adding `validate_resource_specification` method to HTEX and overriding that with MPI specific validation in MPIEX * HTEX will no longer accept mpi_resource_specification even with `enable_mpi_mode=True` * `enable_mpi_mode` and `mpi_launcher` are no longer supported in HTEX. `mpi_launcher` is now set only via MPIEX * MPI specific checks are now in MPIEX instead of HTEX
6aef5dd
to
9a6be96
Compare
Description
This PR moves the following MPI related functionality and options from HTEX to MPIExecutor:
enable_mpi_mode
andmpi_launcher
is now removed from HTEXmpi_launcher
in now in MPIExecutorvalidate_resource_specification
method is added to HTEX that currently asserts that no resource_specification is passed to it, since HTEX does not support any such optionsvalidate_resource_specification
to check for a valid MPI resource specificationThese changes should make it easier to have executor specific resource validation.
Changed Behaviour
enable_mpi_mode
andmpi_launcher
are no longer supported.Type of change
I'm marking this as maintenance since the docs have moved on to only advertize MPI functionality through MPIExecutor.