Skip to content
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

mpi4py as optional dependency #343

Merged
merged 11 commits into from
May 30, 2024
8 changes: 8 additions & 0 deletions .ci_support/environment-mini.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
channels:
- conda-forge
dependencies:
- python
- numpy
- cloudpickle =3.0.0
- tqdm =4.66.4
- pyzmq =26.0.3
32 changes: 32 additions & 0 deletions .github/workflows/minimal.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# This workflow is used to run the unittest of pyiron

name: Unittests-minimal

on:
push:
branches: [ main ]
pull_request:
branches: [ main ]

jobs:
build:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- uses: conda-incubator/[email protected]
with:
python-version: "3.12"
mamba-version: "*"
channels: conda-forge
miniforge-variant: Mambaforge
channel-priority: strict
auto-update-conda: true
environment-file: .ci_support/environment-mini.yml
- name: Test
shell: bash -l {0}
timeout-minutes: 5
run: |
pip install versioneer[toml]==0.29
pip install . --no-deps --no-build-isolation
cd tests
python -m unittest discover .
4 changes: 2 additions & 2 deletions pympipool/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class Executor:
points to the same address as localhost. Still MacOS >= 12 seems to disable
this look up for security reasons. So on MacOS it is required to set this
option to true
backend (str): Switch between the different backends "flux", "mpi" or "slurm". Alternatively, when "auto"
backend (str): Switch between the different backends "flux", "local" or "slurm". Alternatively, when "auto"
is selected (the default) the available backend is determined automatically.
block_allocation (boolean): To accelerate the submission of a series of python functions with the same resource
requirements, pympipool supports block allocation. In this case all resources have
Expand Down Expand Up @@ -138,7 +138,7 @@ def __new__(
points to the same address as localhost. Still MacOS >= 12 seems to disable
this look up for security reasons. So on MacOS it is required to set this
option to true
backend (str): Switch between the different backends "flux", "mpi" or "slurm". Alternatively, when "auto"
backend (str): Switch between the different backends "flux", "local" or "slurm". Alternatively, when "auto"
is selected (the default) the available backend is determined automatically.
block_allocation (boolean): To accelerate the submission of a series of python functions with the same
resource requirements, pympipool supports block allocation. In this case all
Expand Down
4 changes: 2 additions & 2 deletions pympipool/scheduler/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ def create_executor(
any computer should be able to resolve that their own hostname points to the same
address as localhost. Still MacOS >= 12 seems to disable this look up for security
reasons. So on MacOS it is required to set this option to true
backend (str): Switch between the different backends "flux", "mpi" or "slurm". Alternatively, when "auto"
backend (str): Switch between the different backends "flux", "local" or "slurm". Alternatively, when "auto"
is selected (the default) the available backend is determined automatically.
block_allocation (boolean): To accelerate the submission of a series of python functions with the same
resource requirements, pympipool supports block allocation. In this case all
Expand Down Expand Up @@ -145,7 +145,7 @@ def create_executor(
cwd=cwd,
hostname_localhost=hostname_localhost,
)
else: # backend="mpi"
else: # backend="local"
check_threads_per_core(threads_per_core=threads_per_core)
check_gpus_per_worker(gpus_per_worker=gpus_per_worker)
check_command_line_argument_lst(
Expand Down
13 changes: 12 additions & 1 deletion pympipool/shared/executorbase.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,13 @@
check_resource_dict_is_empty,
)

try:
import mpi4py

mpi4py_available = True
except ImportError:
mpi4py_available = False

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider using importlib.util.find_spec instead of a try-except block to check for mpi4py availability. This approach is more explicit and avoids importing the module unnecessarily.

- try:
-     import mpi4py
-     mpi4py_available = True
- except ImportError:
-     mpi4py_available = False
+ from importlib.util import find_spec
+ mpi4py_available = find_spec("mpi4py") is not None

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
try:
import mpi4py
mpi4py_available = True
except ImportError:
mpi4py_available = False
from importlib.util import find_spec
mpi4py_available = find_spec("mpi4py") is not None


class ExecutorBase(FutureExecutor):
def __init__(self):
Expand Down Expand Up @@ -422,8 +429,12 @@ def _get_backend_path(cores: int):
list[str]: List of strings containing the python executable path and the backend script to execute
"""
command_lst = [sys.executable]
if cores > 1:
if cores > 1 and mpi4py_available:
command_lst += [_get_command_path(executable="mpiexec.py")]
elif cores > 1 and not mpi4py_available:
raise ImportError(
"mpi4py is required for parallel calculations. Please install mpi4py."
)
else:
command_lst += [_get_command_path(executable="serial.py")]
return command_lst
Expand Down
6 changes: 3 additions & 3 deletions pympipool/shared/inputcheck.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,9 @@ def check_refresh_rate(refresh_rate: float):
def validate_backend(
backend: str, flux_installed: bool = False, slurm_installed: bool = False
) -> str:
if backend not in ["auto", "mpi", "slurm", "flux"]:
if backend not in ["auto", "flux", "local", "slurm"]:
raise ValueError(
'The currently implemented backends are ["flux", "mpi", "slurm"]. '
'The currently implemented backends are ["auto", "flux", "local", "slurm"]. '
'Alternatively, you can select "auto", the default option, to automatically determine the backend. But '
+ backend
+ " is not a valid choice."
Expand All @@ -90,7 +90,7 @@ def validate_backend(
elif backend == "slurm" or (backend == "auto" and slurm_installed):
return "slurm"
else:
return "mpi"
return "local"


def check_pmi(backend: str, pmi: Optional[str]):
Expand Down
6 changes: 4 additions & 2 deletions pyproject.toml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
[build-system]
requires = ["cloudpickle", "mpi4py", "pyzmq", "setuptools", "tqdm", "versioneer[toml]==0.29"]
requires = ["cloudpickle", "pyzmq", "setuptools", "tqdm", "versioneer[toml]==0.29"]
build-backend = "setuptools.build_meta"

[project]
Expand All @@ -25,7 +25,6 @@ classifiers = [
]
dependencies = [
"cloudpickle==3.0.0",
"mpi4py==3.1.6",
"pyzmq==26.0.3",
"tqdm==4.66.4",
]
Expand All @@ -36,6 +35,9 @@ Homepage = "https://github.com/pyiron/pympipool"
Documentation = "https://pympipool.readthedocs.io"
Repository = "https://github.com/pyiron/pympipool"

[project.optional-dependencies]
mpi = ["mpi4py==3.1.6"]

[tool.setuptools.packages.find]
include = ["pympipool*"]

Expand Down
4 changes: 2 additions & 2 deletions tests/benchmark/llh.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def run_static(mean=0.1, sigma=1.1, runs=32):
sigma=1.1,
runs=32,
max_cores=4,
backend="mpi",
backend="local",
block_allocation=True,
)
elif run_mode == "pympipool":
Expand All @@ -62,7 +62,7 @@ def run_static(mean=0.1, sigma=1.1, runs=32):
sigma=1.1,
runs=32,
max_cores=4,
backend="mpi",
backend="local",
block_allocation=False,
)
elif run_mode == "flux":
Expand Down
2 changes: 1 addition & 1 deletion tests/test_dependencies_executor.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ def add_function(parameter_1, parameter_2):

class TestExecutorWithDependencies(unittest.TestCase):
def test_executor(self):
with Executor(max_cores=1, backend="mpi", hostname_localhost=True) as exe:
with Executor(max_cores=1, backend="local", hostname_localhost=True) as exe:
cloudpickle_register(ind=1)
future_1 = exe.submit(add_function, 1, parameter_2=2)
future_2 = exe.submit(add_function, 1, parameter_2=future_1)
Expand Down
20 changes: 15 additions & 5 deletions tests/test_executor_backend_mpi.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,13 @@
from pympipool import Executor
from pympipool.shared.executorbase import cloudpickle_register

try:
import mpi4py

mpi4py_installed = True
except ImportError:
mpi4py_installed = False

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider using importlib.util.find_spec instead of a direct import to check for mpi4py availability.

- try:
-     import mpi4py
-     mpi4py_installed = True
- except ImportError:
-     mpi4py_installed = False
+ import importlib.util
+ mpi4py_installed = importlib.util.find_spec('mpi4py') is not None

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
try:
import mpi4py
mpi4py_installed = True
except ImportError:
mpi4py_installed = False
import importlib.util
mpi4py_installed = importlib.util.find_spec('mpi4py') is not None


def calc(i):
return i
Expand All @@ -19,7 +26,7 @@ def mpi_funct(i):
class TestExecutorBackend(unittest.TestCase):
def test_meta_executor_serial(self):
with Executor(
max_cores=2, hostname_localhost=True, backend="mpi", block_allocation=True
max_cores=2, hostname_localhost=True, backend="local", block_allocation=True
) as exe:
cloudpickle_register(ind=1)
fs_1 = exe.submit(calc, 1)
Expand All @@ -31,7 +38,7 @@ def test_meta_executor_serial(self):

def test_meta_executor_single(self):
with Executor(
max_cores=1, hostname_localhost=True, backend="mpi", block_allocation=True
max_cores=1, hostname_localhost=True, backend="local", block_allocation=True
) as exe:
cloudpickle_register(ind=1)
fs_1 = exe.submit(calc, 1)
Expand All @@ -41,12 +48,15 @@ def test_meta_executor_single(self):
self.assertTrue(fs_1.done())
self.assertTrue(fs_2.done())

@unittest.skipIf(
mpi4py_installed, "mpi4py is not installed, so the mpi4py tests are skipped."
)
def test_meta_executor_parallel(self):
with Executor(
max_workers=2,
cores_per_worker=2,
hostname_localhost=True,
backend="mpi",
backend="local",
block_allocation=True,
) as exe:
cloudpickle_register(ind=1)
Expand All @@ -61,13 +71,13 @@ def test_errors(self):
cores_per_worker=1,
threads_per_core=2,
hostname_localhost=True,
backend="mpi",
backend="local",
)
with self.assertRaises(TypeError):
Executor(
max_cores=1,
cores_per_worker=1,
gpus_per_worker=1,
hostname_localhost=True,
backend="mpi",
backend="local",
)
26 changes: 12 additions & 14 deletions tests/test_executor_backend_mpi_noblock.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,22 +8,17 @@ def calc(i):
return i


def mpi_funct(i):
from mpi4py import MPI

size = MPI.COMM_WORLD.Get_size()
rank = MPI.COMM_WORLD.Get_rank()
return i, size, rank


def resource_dict(resource_dict):
return resource_dict


class TestExecutorBackend(unittest.TestCase):
def test_meta_executor_serial(self):
with Executor(
max_cores=2, hostname_localhost=True, backend="mpi", block_allocation=False
max_cores=2,
hostname_localhost=True,
backend="local",
block_allocation=False,
) as exe:
cloudpickle_register(ind=1)
fs_1 = exe.submit(calc, 1)
Expand All @@ -35,7 +30,10 @@ def test_meta_executor_serial(self):

def test_meta_executor_single(self):
with Executor(
max_cores=1, hostname_localhost=True, backend="mpi", block_allocation=False
max_cores=1,
hostname_localhost=True,
backend="local",
block_allocation=False,
) as exe:
cloudpickle_register(ind=1)
fs_1 = exe.submit(calc, 1)
Expand All @@ -52,29 +50,29 @@ def test_errors(self):
cores_per_worker=1,
threads_per_core=2,
hostname_localhost=True,
backend="mpi",
backend="local",
)
with self.assertRaises(TypeError):
Executor(
max_cores=1,
cores_per_worker=1,
gpus_per_worker=1,
hostname_localhost=True,
backend="mpi",
backend="local",
)
with self.assertRaises(ValueError):
with Executor(
max_cores=1,
hostname_localhost=True,
backend="mpi",
backend="local",
block_allocation=False,
) as exe:
exe.submit(resource_dict, resource_dict={})
with self.assertRaises(ValueError):
with Executor(
max_cores=1,
hostname_localhost=True,
backend="mpi",
backend="local",
block_allocation=True,
) as exe:
exe.submit(resource_dict, resource_dict={})
19 changes: 19 additions & 0 deletions tests/test_mpi_executor.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,13 @@
ExecutorBase,
)

try:
import mpi4py

mpi4py_installed = True
except ImportError:
mpi4py_installed = False

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider using importlib.util.find_spec instead of a try-except block to check for mpi4py availability. This approach is more explicit and avoids importing the module unnecessarily.

- try:
-     import mpi4py
-     mpi4py_installed = True
- except ImportError:
-     mpi4py_installed = False
+ from importlib.util import find_spec
+ mpi4py_installed = find_spec("mpi4py") is not None

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
try:
import mpi4py
mpi4py_installed = True
except ImportError:
mpi4py_installed = False
from importlib.util import find_spec
mpi4py_installed = find_spec("mpi4py") is not None


def calc(i):
return i
Expand Down Expand Up @@ -127,6 +134,9 @@ def test_pympiexecutor_errors(self):
)


@unittest.skipIf(
mpi4py_installed, "mpi4py is not installed, so the mpi4py tests are skipped."
)
class TestPyMpiExecutorMPI(unittest.TestCase):
def test_pympiexecutor_one_worker_with_mpi(self):
with PyMPIExecutor(
Expand Down Expand Up @@ -164,6 +174,9 @@ def test_pympiexecutor_one_worker_with_mpi_echo(self):
self.assertEqual(output, [2, 2])


@unittest.skipIf(
mpi4py_installed, "mpi4py is not installed, so the mpi4py tests are skipped."
)
class TestPyMpiStepExecutorMPI(unittest.TestCase):
def test_pympiexecutor_one_worker_with_mpi(self):
with PyMPIStepExecutor(
Expand Down Expand Up @@ -339,6 +352,9 @@ def test_meta_step(self):
else:
self.assertEqual(str(exe.info[k]), v)

@unittest.skipIf(
mpi4py_installed, "mpi4py is not installed, so the mpi4py tests are skipped."
)
def test_pool_multi_core(self):
with PyMPIExecutor(
max_workers=1, cores_per_worker=2, hostname_localhost=True
Expand All @@ -352,6 +368,9 @@ def test_pool_multi_core(self):
self.assertEqual(len(p), 0)
self.assertEqual(output.result(), [(2, 2, 0), (2, 2, 1)])

@unittest.skipIf(
mpi4py_installed, "mpi4py is not installed, so the mpi4py tests are skipped."
)
def test_pool_multi_core_map(self):
with PyMPIExecutor(
max_workers=1, cores_per_worker=2, hostname_localhost=True
Expand Down
Loading
Loading