Skip to content

Commit

Permalink
Make HTEX reject empty resource specification in MPI mode (#3442)
Browse files Browse the repository at this point in the history
Fixes #3427
  • Loading branch information
R1j1t authored May 27, 2024
1 parent a2ea9cc commit 11664da
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 12 deletions.
3 changes: 2 additions & 1 deletion parsl/executors/high_throughput/executor.py
Original file line number Diff line number Diff line change
Expand Up @@ -645,7 +645,8 @@ def submit(self, func, resource_specification, *args, **kwargs):
Returns:
Future
"""
validate_resource_spec(resource_specification)

validate_resource_spec(resource_specification, self.enable_mpi_mode)

if self.bad_state_is_set:
raise self.executor_exception
Expand Down
20 changes: 18 additions & 2 deletions parsl/executors/high_throughput/mpi_prefix_composer.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,18 @@
'mpiexec')


class MissingResourceSpecification(Exception):
"""Exception raised when input is not supplied a resource specification"""

def __init__(self, reason: str):
self.reason = reason

def __str__(self):
return f"Missing resource specification: {self.reason}"


class InvalidResourceSpecification(Exception):
"""Exception raised when Invalid keys are supplied via resource specification"""
"""Exception raised when Invalid input is supplied via resource specification"""

def __init__(self, invalid_keys: Set[str]):
self.invalid_keys = invalid_keys
Expand All @@ -18,13 +28,19 @@ def __str__(self):
return f"Invalid resource specification options supplied: {self.invalid_keys}"


def validate_resource_spec(resource_spec: Dict[str, str]):
def validate_resource_spec(resource_spec: Dict[str, str], is_mpi_enabled: bool):
"""Basic validation of keys in the resource_spec
Raises: InvalidResourceSpecification if the resource_spec
is invalid (e.g, contains invalid keys)
"""
user_keys = set(resource_spec.keys())

# empty resource_spec when mpi_mode is set causes parsl to hang
# ref issue #3427
if is_mpi_enabled and len(user_keys) == 0:
raise MissingResourceSpecification('MPI mode requires optional parsl_resource_specification keyword argument to be configured')

legal_keys = set(("ranks_per_node",
"num_nodes",
"num_ranks",
Expand Down
10 changes: 10 additions & 0 deletions parsl/tests/test_mpi_apps/test_mpi_mode_enabled.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
from parsl import python_app, bash_app
from parsl.tests.configs.htex_local import fresh_config

from parsl.executors.high_throughput.mpi_prefix_composer import MissingResourceSpecification

import os

EXECUTOR_LABEL = "MPI_TEST"
Expand Down Expand Up @@ -168,3 +170,11 @@ def test_simulated_load(rounds: int = 100):
total_ranks, nodes = future.result(timeout=10)
assert len(nodes) == futures[future]["num_nodes"]
assert total_ranks == futures[future]["num_nodes"] * futures[future]["ranks_per_node"]


@pytest.mark.local
def test_missing_resource_spec():

with pytest.raises(MissingResourceSpecification):
future = mock_app(sleep_dur=0.4)
future.result(timeout=10)
23 changes: 14 additions & 9 deletions parsl/tests/test_mpi_apps/test_resource_spec.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@
)
from parsl.executors.high_throughput.mpi_prefix_composer import (
validate_resource_spec,
InvalidResourceSpecification
InvalidResourceSpecification,
MissingResourceSpecification
)

EXECUTOR_LABEL = "MPI_TEST"
Expand Down Expand Up @@ -122,18 +123,22 @@ def test_top_level():

@pytest.mark.local
@pytest.mark.parametrize(
"resource_spec, exception",
"resource_spec, is_mpi_enabled, exception",
(
({"num_nodes": 2, "ranks_per_node": 1}, None),
({"launcher_options": "--debug_foo"}, None),
({"num_nodes": 2, "BAD_OPT": 1}, InvalidResourceSpecification),
({}, None),
({"num_nodes": 2, "ranks_per_node": 1}, False, None),
({"launcher_options": "--debug_foo"}, False, None),
({"num_nodes": 2, "BAD_OPT": 1}, False, InvalidResourceSpecification),
({}, False, None),
({"num_nodes": 2, "ranks_per_node": 1}, True, None),
({"launcher_options": "--debug_foo"}, True, None),
({"num_nodes": 2, "BAD_OPT": 1}, True, InvalidResourceSpecification),
({}, True, MissingResourceSpecification),
)
)
def test_resource_spec(resource_spec: Dict, exception):
def test_resource_spec(resource_spec: Dict, is_mpi_enabled: bool, exception):
if exception:
with pytest.raises(exception):
validate_resource_spec(resource_spec)
validate_resource_spec(resource_spec, is_mpi_enabled)
else:
result = validate_resource_spec(resource_spec)
result = validate_resource_spec(resource_spec, is_mpi_enabled)
assert result is None

0 comments on commit 11664da

Please sign in to comment.