Skip to content

Remove conda support in executorlib #418

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Oct 1, 2024
Merged

Remove conda support in executorlib #418

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
76291be
Remove conda support in executorlib
jan-janssen Oct 1, 2024
0fecee8
Merge remote-tracking branch 'origin/main' into remove_conda
jan-janssen Oct 1, 2024
6d20869
Merge remote-tracking branch 'origin/main' into remove_conda
jan-janssen Oct 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .ci_support/environment-mpich.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ dependencies:
- numpy
- mpich
- cloudpickle =3.0.0
- conda_subprocess =0.0.5
- mpi4py =4.0.0
- pyzmq =26.2.0
- h5py =3.11.0
Expand Down
1 change: 0 additions & 1 deletion .ci_support/environment-old.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ dependencies:
- numpy
- openmpi =4.1.4
- cloudpickle =2.0.0
- conda_subprocess =0.0.3
- mpi4py =3.1.4
- pyzmq =25.0.0
- h5py =3.6.0
Expand Down
1 change: 0 additions & 1 deletion .ci_support/environment-openmpi.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ dependencies:
- numpy
- openmpi
- cloudpickle =3.0.0
- conda_subprocess =0.0.5
- mpi4py =4.0.0
- pyzmq =26.2.0
- h5py =3.11.0
Expand Down
1 change: 0 additions & 1 deletion .ci_support/environment-win.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ dependencies:
- numpy
- msmpi
- cloudpickle =3.0.0
- conda_subprocess =0.0.5
- mpi4py =4.0.0
- pyzmq =26.2.0
- h5py =3.11.0
Expand Down
1 change: 0 additions & 1 deletion .github/workflows/unittest-flux-mpich.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ jobs:
shell: bash -l {0}
timeout-minutes: 5
run: |
conda create -y -n py312 python=3.12.1 executorlib
pip install . --no-deps --no-build-isolation
python -m unittest discover tests
- name: Test Flux
Expand Down
1 change: 0 additions & 1 deletion .github/workflows/unittest-flux-openmpi.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ jobs:
shell: bash -l {0}
timeout-minutes: 5
run: |
conda create -y -n py312 python=3.12.1 executorlib
pip install . --no-deps --no-build-isolation
coverage run -a --omit="executorlib/_version.py,tests/*" -m unittest discover tests
- name: Test Flux with OpenMPI
Expand Down
1 change: 0 additions & 1 deletion .github/workflows/unittest-mpich.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ jobs:
timeout-minutes: 5
run: |
pip install versioneer[toml]==0.29
conda create -y -n py312 python=3.12.1 executorlib
pip install . --no-deps --no-build-isolation
cd tests
python -m unittest discover .
1 change: 0 additions & 1 deletion .github/workflows/unittest-openmpi.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ jobs:
timeout-minutes: 5
run: |
pip install versioneer[toml]==0.29
conda create -y -n py312 python=3.12.1 executorlib
pip install . --no-deps --no-build-isolation
cd tests
python -m unittest discover .
1 change: 0 additions & 1 deletion .github/workflows/unittest-win.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ jobs:
timeout-minutes: 5
run: |
pip install versioneer[toml]==0.29
conda create -y -n py312 python=3.12.1 executorlib
pip install . --no-deps --no-build-isolation
cd tests
python -m unittest discover .
1 change: 0 additions & 1 deletion .github/workflows/unittests-old.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ jobs:
timeout-minutes: 5
run: |
pip install versioneer[toml]==0.29
conda create -y -n py312 python=3.12.1 executorlib
pip install . --no-deps --no-build-isolation
cd tests
python -m unittest discover .
Expand Down
2 changes: 1 addition & 1 deletion .readthedocs.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,6 @@ sphinx:
# Optionally build your docs in additional formats such as PDF and ePub
formats: []

# Install pyiron from conda
# Install executorlib from conda
conda:
environment: .ci_support/environment-docs.yml
Comment on lines 29 to 30
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 1, 2024

Choose a reason for hiding this comment

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

💡 Codebase verification

Conda Configurations Still Present

Conda configurations are still found in the following files and should be removed to align with the PR's objective of eliminating conda support:

  • binder/environment.yml
  • .readthedocs.yml
🔗 Analysis chain

Verify: Is conda configuration still needed?

The PR objectives mention removing conda support, but this file still includes conda configuration. Please confirm if this is intentional or if the conda-related lines should be removed as part of this PR.

To help verify this, you can run the following command to check for other conda-related configurations in the project:

This will help determine if conda configurations have been removed from other files, which might indicate that this file was overlooked.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for conda-related configurations in other files
rg --type yaml --type toml 'conda' -g '!.readthedocs.yml'

Length of output: 94

12 changes: 0 additions & 12 deletions executorlib/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,6 @@ class Executor:
flux_executor (flux.job.FluxExecutor): Flux Python interface to submit the workers to flux
flux_executor_pmi_mode (str): PMI interface to use (OpenMPI v5 requires pmix) default is None (Flux only)
flux_executor_nesting (bool): Provide hierarchically nested Flux job scheduler inside the submitted function.
conda_environment_name (str): name of the conda environment to initialize
conda_environment_path (str): path of the conda environment to initialize
hostname_localhost (boolean): use localhost instead of the hostname to establish the zmq connection. In the
context of an HPC cluster this essential to be able to communicate to an
Executor running on a different compute node within the same allocation. And
Expand Down Expand Up @@ -105,8 +103,6 @@ def __init__(
flux_executor=None,
flux_executor_pmi_mode: Optional[str] = None,
flux_executor_nesting: bool = False,
conda_environment_name: Optional[str] = None,
conda_environment_path: Optional[str] = None,
hostname_localhost: bool = False,
block_allocation: bool = True,
init_function: Optional[callable] = None,
Expand All @@ -131,8 +127,6 @@ def __new__(
flux_executor=None,
flux_executor_pmi_mode: Optional[str] = None,
flux_executor_nesting: bool = False,
conda_environment_name: Optional[str] = None,
conda_environment_path: Optional[str] = None,
hostname_localhost: bool = False,
block_allocation: bool = True,
init_function: Optional[callable] = None,
Expand Down Expand Up @@ -163,8 +157,6 @@ def __new__(
flux_executor (flux.job.FluxExecutor): Flux Python interface to submit the workers to flux
flux_executor_pmi_mode (str): PMI interface to use (OpenMPI v5 requires pmix) default is None (Flux only)
flux_executor_nesting (bool): Provide hierarchically nested Flux job scheduler inside the submitted function.
conda_environment_name (str): name of the conda environment to initialize
conda_environment_path (str): path of the conda environment to initialize
hostname_localhost (boolean): use localhost instead of the hostname to establish the zmq connection. In the
context of an HPC cluster this essential to be able to communicate to an
Executor running on a different compute node within the same allocation. And
Expand Down Expand Up @@ -197,8 +189,6 @@ def __new__(
flux_executor=flux_executor,
flux_executor_pmi_mode=flux_executor_pmi_mode,
flux_executor_nesting=flux_executor_nesting,
conda_environment_name=conda_environment_name,
conda_environment_path=conda_environment_path,
hostname_localhost=hostname_localhost,
block_allocation=block_allocation,
init_function=init_function,
Expand All @@ -221,8 +211,6 @@ def __new__(
flux_executor=flux_executor,
flux_executor_pmi_mode=flux_executor_pmi_mode,
flux_executor_nesting=flux_executor_nesting,
conda_environment_name=conda_environment_name,
conda_environment_path=conda_environment_path,
hostname_localhost=hostname_localhost,
block_allocation=block_allocation,
init_function=init_function,
Expand Down
6 changes: 0 additions & 6 deletions executorlib/interactive/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,6 @@ def create_executor(
flux_executor=None,
flux_executor_pmi_mode: Optional[str] = None,
flux_executor_nesting: bool = False,
conda_environment_name: Optional[str] = None,
conda_environment_path: Optional[str] = None,
hostname_localhost: bool = False,
block_allocation: bool = False,
init_function: Optional[callable] = None,
Expand Down Expand Up @@ -69,8 +67,6 @@ def create_executor(
flux_executor (flux.job.FluxExecutor): Flux Python interface to submit the workers to flux
flux_executor_pmi_mode (str): PMI interface to use (OpenMPI v5 requires pmix) default is None (Flux only)
flux_executor_nesting (bool): Provide hierarchically nested Flux job scheduler inside the submitted function.
conda_environment_name (str): name of the conda environment to initialize
conda_environment_path (str): path of the conda environment to initialize
hostname_localhost (boolean): use localhost instead of the hostname to establish the zmq connection. In the
context of an HPC cluster this essential to be able to communicate to an Executor
running on a different compute node within the same allocation. And in principle
Expand All @@ -92,8 +88,6 @@ def create_executor(
"cores": cores_per_worker,
"hostname_localhost": hostname_localhost,
"cwd": cwd,
"conda_environment_name": conda_environment_name,
"conda_environment_path": conda_environment_path,
}
if backend == "flux":
check_oversubscribe(oversubscribe=openmpi_oversubscribe)
Expand Down
8 changes: 0 additions & 8 deletions executorlib/interactive/flux.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,27 +47,19 @@ def __init__(
def bootup(
self,
command_lst: list[str],
conda_environment_name: Optional[str] = None,
conda_environment_path: Optional[str] = None,
):
"""
Boot up the client process to connect to the SocketInterface.

Args:
command_lst (list[str]): List of strings to start the client process.
conda_environment_name (str, optional): Name of the conda environment to initialize. Defaults to None.
conda_environment_path (str, optional): Path of the conda environment to initialize. Defaults to None.
Raises:
ValueError: If oversubscribing is not supported for the Flux adapter or if conda environments are not supported.
"""
if self._openmpi_oversubscribe:
raise ValueError(
"Oversubscribing is currently not supported for the Flux adapter."
)
if conda_environment_name is not None or conda_environment_path is not None:
raise ValueError(
"Conda environments are currently not supported for the Flux adapter."
)
if self._flux_executor is None:
self._flux_executor = flux.job.FluxExecutor()
if not self._flux_executor_nesting:
Expand Down
13 changes: 0 additions & 13 deletions executorlib/shared/communication.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
from socket import gethostname
from typing import Optional

import cloudpickle
import zmq
Expand Down Expand Up @@ -76,21 +75,15 @@ def bind_to_random_port(self):
def bootup(
self,
command_lst: list[str],
conda_environment_name: Optional[str] = None,
conda_environment_path: Optional[str] = None,
):
"""
Boot up the client process to connect to the SocketInterface.

Args:
command_lst (list): list of strings to start the client process
conda_environment_name (str): name of the conda environment to initialize
conda_environment_path (str): path of the conda environment to initialize
"""
self._spawner.bootup(
command_lst=command_lst,
conda_environment_name=conda_environment_name,
conda_environment_path=conda_environment_path,
)

def shutdown(self, wait: bool = True):
Expand Down Expand Up @@ -127,8 +120,6 @@ def interface_bootup(
command_lst: list[str],
connections,
hostname_localhost: bool = False,
conda_environment_name: Optional[str] = None,
conda_environment_path: Optional[str] = None,
):
"""
Start interface for ZMQ communication
Expand All @@ -144,8 +135,6 @@ def interface_bootup(
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
conda_environment_name (str): name of the conda environment to initialize
conda_environment_path (str): path of the conda environment to initialize

Returns:
executorlib.shared.communication.SocketInterface: socket interface for zmq communication
Expand All @@ -162,8 +151,6 @@ def interface_bootup(
]
interface.bootup(
command_lst=command_lst,
conda_environment_name=conda_environment_name,
conda_environment_path=conda_environment_path,
)
return interface

Expand Down
6 changes: 0 additions & 6 deletions executorlib/shared/executor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,8 +304,6 @@ def execute_parallel_tasks(
spawner: BaseSpawner = MpiExecSpawner,
hostname_localhost: bool = False,
init_function: Optional[Callable] = None,
conda_environment_name: Optional[str] = None,
conda_environment_path: Optional[str] = None,
**kwargs,
) -> None:
"""
Expand All @@ -323,17 +321,13 @@ def execute_parallel_tasks(
this look up for security reasons. So on MacOS it is required to set this
option to true
init_function (callable): optional function to preset arguments for functions which are submitted later
conda_environment_name (str): name of the conda environment to initialize
conda_environment_path (str): path of the conda environment to initialize
"""
interface = interface_bootup(
command_lst=_get_backend_path(
cores=cores,
),
connections=spawner(cores=cores, **kwargs),
hostname_localhost=hostname_localhost,
conda_environment_path=conda_environment_path,
conda_environment_name=conda_environment_name,
)
if init_function is not None:
interface.send_dict(
Expand Down
29 changes: 5 additions & 24 deletions executorlib/shared/spawner.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,12 @@ def __init__(self, cwd: str, cores: int = 1, openmpi_oversubscribe: bool = False
def bootup(
self,
command_lst: list[str],
conda_environment_name: Optional[str] = None,
conda_environment_path: Optional[str] = None,
):
"""
Method to start the interface.

Args:
command_lst (list[str]): The command list to execute.
conda_environment_name (str, optional): The prefix name. Defaults to None.
conda_environment_path (str, optional): The prefix path. Defaults to None.
"""
raise NotImplementedError

Expand Down Expand Up @@ -80,33 +76,18 @@ def __init__(
def bootup(
self,
command_lst: list[str],
conda_environment_name: Optional[str] = None,
conda_environment_path: Optional[str] = None,
):
"""
Method to start the subprocess interface.

Args:
command_lst (list[str]): The command list to execute.
conda_environment_name (str, optional): The prefix name. Defaults to None.
conda_environment_path (str, optional): The prefix path. Defaults to None.
"""
if conda_environment_name is None and conda_environment_path is None:
self._process = subprocess.Popen(
args=self.generate_command(command_lst=command_lst),
cwd=self._cwd,
stdin=subprocess.DEVNULL,
)
else:
import conda_subprocess

self._process = conda_subprocess.Popen(
args=self.generate_command(command_lst=command_lst),
cwd=self._cwd,
stdin=subprocess.DEVNULL,
prefix_path=conda_environment_path,
prefix_name=conda_environment_name,
)
self._process = subprocess.Popen(
args=self.generate_command(command_lst=command_lst),
cwd=self._cwd,
stdin=subprocess.DEVNULL,
)
Comment on lines +86 to +90
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 1, 2024

Choose a reason for hiding this comment

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

💡 Codebase verification

Found remaining conda references.

There is still a reference to conda in executorlib/interactive/flux.py. Please remove this reference to fully eliminate conda support.

🔗 Analysis chain

LGTM! Consider adding a comment about conda support removal.

The simplification of the bootup method aligns with the PR objective of removing conda support. This change improves code readability and maintainability.

Consider adding a comment explaining that conda support has been removed and users should decorate their functions if they need conda support:

def bootup(
    self,
    command_lst: list[str],
):
    """
    Method to start the subprocess interface.

    Note: Conda support has been removed. Users requiring conda support
    should decorate their functions before submitting them through executorlib.

    Args:
        command_lst (list[str]): The command list to execute.
    """
    self._process = subprocess.Popen(
        args=self.generate_command(command_lst=command_lst),
        cwd=self._cwd,
        stdin=subprocess.DEVNULL,
    )

Let's verify the impact of these changes on the rest of the codebase:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any remaining references to conda in the codebase
# and verify that all calls to bootup method have been updated.

echo "Checking for remaining conda references:"
rg -i conda --type python

echo "\nVerifying bootup method calls:"
rg -A 3 "\.bootup\(" --type python

Length of output: 380

Script:

#!/bin/bash
# Description: Check for any remaining references to conda in the codebase
# and verify that all calls to bootup method have been updated.

echo "Checking for remaining conda references:"
rg -i conda --type py

echo "\nVerifying bootup method calls:"
rg -A 3 "\.bootup\(" --type py

Length of output: 1431


def generate_command(self, command_lst: list[str]) -> list[str]:
"""
Expand Down
Loading