Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 5 additions & 5 deletions executorlib/executor/flux.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class FluxJobExecutor(BaseExecutor):
max_workers (int): for backwards compatibility with the standard library, max_workers also defines the number of
cores which can be used in parallel - just like the max_cores parameter. Using max_cores is
recommended, as computers have a limited number of compute cores.
cache_directory (str, optional): The directory to store cache files. Defaults to "cache".
cache_directory (str, optional): The directory to store cache files. Defaults to "executorlib_cache".
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Docstring says "executorlib_cache" is the default, but the code still defaults to None

All affected docstrings advertise cache_directory defaults to "executorlib_cache", yet every public signature (FluxJobExecutor.__init__, FluxClusterExecutor.__init__, create_flux_executor) still uses cache_directory: Optional[str] = None.
This discrepancy will confuse users and downstream tooling and can silently break workloads that rely on the advertised default.

Proposed quick fix – either update the signature or introduce a fallback inside create_flux_executor (preferred to keep one authoritative place):

@@
-    cache_directory: Optional[str] = None,
+    cache_directory: Optional[str] = "executorlib_cache",

or

@@
 if resource_dict is None:
     resource_dict = {}
+
+# honour advertised default
+if cache_directory is None:
+    cache_directory = "executorlib_cache"

Please align implementation and documentation across the whole codebase before releasing.

Also applies to: 117-117, 221-221, 297-297, 412-412


🏁 Script executed:

#!/bin/bash
# Locate remaining occurrences where the parameter still defaults to None
rg --line-number $'cache_directory: Optional\\[str\\] = None'

Length of output: 1078


Unify cache_directory default to "executorlib_cache" across the codebase

Docstrings currently state that cache_directory defaults to "executorlib_cache", but all public constructors and factory functions still use cache_directory: Optional[str] = None. This mismatch can confuse users and tooling, and may silently break downstream workloads.

Please update every occurrence of

cache_directory: Optional[str] = None

to either:

  1. Change the signature default
    - cache_directory: Optional[str] = None
    + cache_directory: Optional[str] = "executorlib_cache"
  2. Or add a single fallback in each factory function (preferred if you’d rather keep one authoritative place):
    if cache_directory is None:
        cache_directory = "executorlib_cache"

Apply this change at all of the following locations:

  • executorlib/task_scheduler/interactive/shared.py:24
  • executorlib/task_scheduler/file/queue_spawner.py:18
  • executorlib/task_scheduler/file/subprocess_spawner.py:15
  • executorlib/executor/slurm.py:86, 267, 377
  • executorlib/executor/single.py:84, 190
  • executorlib/executor/flux.py:90, 274, 393
  • executorlib/task_scheduler/file/task_scheduler.py:88

After updating, verify that every docstring and default value are in sync before releasing.

🤖 Prompt for AI Agents
In executorlib/executor/flux.py at lines 32, 90, 274, and 393, the parameter
cache_directory is documented to default to "executorlib_cache" but the function
signatures still default it to None. To fix this, add a fallback inside each
relevant factory function or constructor so that if cache_directory is None, it
is set to "executorlib_cache". This keeps one authoritative default and aligns
the implementation with the docstrings, preventing confusion and potential
silent failures.

max_cores (int): defines the number cores which can be used in parallel
resource_dict (dict): A dictionary of resources required by the task. With the following keys:
- cores (int): number of MPI cores to be used for each function call
Expand Down Expand Up @@ -114,7 +114,7 @@ def __init__(
max_workers (int): for backwards compatibility with the standard library, max_workers also defines the
number of cores which can be used in parallel - just like the max_cores parameter. Using
max_cores is recommended, as computers have a limited number of compute cores.
cache_directory (str, optional): The directory to store cache files. Defaults to "cache".
cache_directory (str, optional): The directory to store cache files. Defaults to "executorlib_cache".
max_cores (int): defines the number cores which can be used in parallel
resource_dict (dict): A dictionary of resources required by the task. With the following keys:
- cores (int): number of MPI cores to be used for each function call
Expand Down Expand Up @@ -218,7 +218,7 @@ class FluxClusterExecutor(BaseExecutor):
max_workers (int): for backwards compatibility with the standard library, max_workers also defines the number of
cores which can be used in parallel - just like the max_cores parameter. Using max_cores is
recommended, as computers have a limited number of compute cores.
cache_directory (str, optional): The directory to store cache files. Defaults to "cache".
cache_directory (str, optional): The directory to store cache files. Defaults to "executorlib_cache".
max_cores (int): defines the number cores which can be used in parallel
resource_dict (dict): A dictionary of resources required by the task. With the following keys:
- cores (int): number of MPI cores to be used for each function call
Expand Down Expand Up @@ -294,7 +294,7 @@ def __init__(
max_workers (int): for backwards compatibility with the standard library, max_workers also defines the
number of cores which can be used in parallel - just like the max_cores parameter. Using
max_cores is recommended, as computers have a limited number of compute cores.
cache_directory (str, optional): The directory to store cache files. Defaults to "cache".
cache_directory (str, optional): The directory to store cache files. Defaults to "executorlib_cache".
max_cores (int): defines the number cores which can be used in parallel
resource_dict (dict): A dictionary of resources required by the task. With the following keys:
- cores (int): number of MPI cores to be used for each function call
Expand Down Expand Up @@ -409,7 +409,7 @@ def create_flux_executor(
number of cores which can be used in parallel - just like the max_cores parameter. Using
max_cores is recommended, as computers have a limited number of compute cores.
max_cores (int): defines the number cores which can be used in parallel
cache_directory (str, optional): The directory to store cache files. Defaults to "cache".
cache_directory (str, optional): The directory to store cache files. Defaults to "executorlib_cache".
resource_dict (dict): A dictionary of resources required by the task. With the following keys:
- cores (int): number of MPI cores to be used for each function call
- threads_per_core (int): number of OpenMP threads to be used for each function call
Expand Down
6 changes: 3 additions & 3 deletions executorlib/executor/single.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class SingleNodeExecutor(BaseExecutor):
max_workers (int): for backwards compatibility with the standard library, max_workers also defines the number of
cores which can be used in parallel - just like the max_cores parameter. Using max_cores is
recommended, as computers have a limited number of compute cores.
cache_directory (str, optional): The directory to store cache files. Defaults to "cache".
cache_directory (str, optional): The directory to store cache files. Defaults to "executorlib_cache".
max_cores (int): defines the number cores which can be used in parallel
resource_dict (dict): A dictionary of resources required by the task. With the following keys:
- cores (int): number of MPI cores to be used for each function call
Expand Down Expand Up @@ -104,7 +104,7 @@ def __init__(
max_workers (int): for backwards compatibility with the standard library, max_workers also defines the
number of cores which can be used in parallel - just like the max_cores parameter. Using
max_cores is recommended, as computers have a limited number of compute cores.
cache_directory (str, optional): The directory to store cache files. Defaults to "cache".
cache_directory (str, optional): The directory to store cache files. Defaults to "executorlib_cache".
max_cores (int): defines the number cores which can be used in parallel
resource_dict (dict): A dictionary of resources required by the task. With the following keys:
- cores (int): number of MPI cores to be used for each function call
Expand Down Expand Up @@ -202,7 +202,7 @@ def create_single_node_executor(
number of cores which can be used in parallel - just like the max_cores parameter. Using
max_cores is recommended, as computers have a limited number of compute cores.
max_cores (int): defines the number cores which can be used in parallel
cache_directory (str, optional): The directory to store cache files. Defaults to "cache".
cache_directory (str, optional): The directory to store cache files. Defaults to "executorlib_cache".
resource_dict (dict): A dictionary of resources required by the task. With the following keys:
- cores (int): number of MPI cores to be used for each function call
- threads_per_core (int): number of OpenMP threads to be used for each function call
Expand Down
10 changes: 5 additions & 5 deletions executorlib/executor/slurm.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class SlurmClusterExecutor(BaseExecutor):
max_workers (int): for backwards compatibility with the standard library, max_workers also defines the number of
cores which can be used in parallel - just like the max_cores parameter. Using max_cores is
recommended, as computers have a limited number of compute cores.
cache_directory (str, optional): The directory to store cache files. Defaults to "cache".
cache_directory (str, optional): The directory to store cache files. Defaults to "executorlib_cache".
max_cores (int): defines the number cores which can be used in parallel
resource_dict (dict): A dictionary of resources required by the task. With the following keys:
- cores (int): number of MPI cores to be used for each function call
Expand Down Expand Up @@ -106,7 +106,7 @@ def __init__(
max_workers (int): for backwards compatibility with the standard library, max_workers also defines the
number of cores which can be used in parallel - just like the max_cores parameter. Using
max_cores is recommended, as computers have a limited number of compute cores.
cache_directory (str, optional): The directory to store cache files. Defaults to "cache".
cache_directory (str, optional): The directory to store cache files. Defaults to "executorlib_cache".
max_cores (int): defines the number cores which can be used in parallel
resource_dict (dict): A dictionary of resources required by the task. With the following keys:
- cores (int): number of MPI cores to be used for each function call
Expand Down Expand Up @@ -207,7 +207,7 @@ class SlurmJobExecutor(BaseExecutor):
max_workers (int): for backwards compatibility with the standard library, max_workers also defines the number of
cores which can be used in parallel - just like the max_cores parameter. Using max_cores is
recommended, as computers have a limited number of compute cores.
cache_directory (str, optional): The directory to store cache files. Defaults to "cache".
cache_directory (str, optional): The directory to store cache files. Defaults to "executorlib_cache".
max_cores (int): defines the number cores which can be used in parallel
resource_dict (dict): A dictionary of resources required by the task. With the following keys:
- cores (int): number of MPI cores to be used for each function call
Expand Down Expand Up @@ -287,7 +287,7 @@ def __init__(
max_workers (int): for backwards compatibility with the standard library, max_workers also defines the
number of cores which can be used in parallel - just like the max_cores parameter. Using
max_cores is recommended, as computers have a limited number of compute cores.
cache_directory (str, optional): The directory to store cache files. Defaults to "cache".
cache_directory (str, optional): The directory to store cache files. Defaults to "executorlib_cache".
max_cores (int): defines the number cores which can be used in parallel
resource_dict (dict): A dictionary of resources required by the task. With the following keys:
- cores (int): number of MPI cores to be used for each function call
Expand Down Expand Up @@ -389,7 +389,7 @@ def create_slurm_executor(
number of cores which can be used in parallel - just like the max_cores parameter. Using
max_cores is recommended, as computers have a limited number of compute cores.
max_cores (int): defines the number cores which can be used in parallel
cache_directory (str, optional): The directory to store cache files. Defaults to "cache".
cache_directory (str, optional): The directory to store cache files. Defaults to "executorlib_cache".
resource_dict (dict): A dictionary of resources required by the task. With the following keys:
- cores (int): number of MPI cores to be used for each function call
- threads_per_core (int): number of OpenMP threads to be used for each function call
Expand Down
4 changes: 2 additions & 2 deletions executorlib/task_scheduler/file/task_scheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
class FileTaskScheduler(TaskSchedulerBase):
def __init__(
self,
cache_directory: str = "cache",
cache_directory: str = "executorlib_cache",
resource_dict: Optional[dict] = None,
execute_function: Callable = execute_with_pysqa,
terminate_function: Optional[Callable] = None,
Expand All @@ -39,7 +39,7 @@ def __init__(
Initialize the FileExecutor.

Args:
cache_directory (str, optional): The directory to store cache files. Defaults to "cache".
cache_directory (str, optional): The directory to store cache files. Defaults to "executorlib_cache".
resource_dict (dict): A dictionary of resources required by the task. With the following keys:
- cores (int): number of MPI cores to be used for each function call
- cwd (str/None): current working directory where the parallel python task is executed
Expand Down
2 changes: 1 addition & 1 deletion executorlib/task_scheduler/interactive/shared.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def execute_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
cache_directory (str, optional): The directory to store cache files. Defaults to "cache".
cache_directory (str, optional): The directory to store cache files. Defaults to "executorlib_cache".
queue_join_on_shutdown (bool): Join communication queue when thread is closed. Defaults to True.
log_obj_size (bool): Enable debug mode which reports the size of the communicated objects.
"""
Expand Down
11 changes: 5 additions & 6 deletions tests/test_cache_backend_execute.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ def get_error(a):
)
class TestSharedFunctions(unittest.TestCase):
def test_execute_function_mixed(self):
cache_directory = os.path.abspath("cache")
cache_directory = os.path.abspath("executorlib_cache")
os.makedirs(cache_directory, exist_ok=True)
task_key, data_dict = serialize_funct_h5(
fn=my_funct,
Expand Down Expand Up @@ -56,7 +56,7 @@ def test_execute_function_mixed(self):
self.assertEqual(future_file_obj.result(), 3)

def test_execute_function_args(self):
cache_directory = os.path.abspath("cache")
cache_directory = os.path.abspath("executorlib_cache")
os.makedirs(cache_directory, exist_ok=True)
task_key, data_dict = serialize_funct_h5(
fn=my_funct,
Expand Down Expand Up @@ -84,7 +84,7 @@ def test_execute_function_args(self):
self.assertEqual(future_file_obj.result(), 3)

def test_execute_function_kwargs(self):
cache_directory = os.path.abspath("cache")
cache_directory = os.path.abspath("executorlib_cache")
os.makedirs(cache_directory, exist_ok=True)
task_key, data_dict = serialize_funct_h5(
fn=my_funct,
Expand Down Expand Up @@ -112,7 +112,7 @@ def test_execute_function_kwargs(self):
self.assertEqual(future_file_obj.result(), 3)

def test_execute_function_error(self):
cache_directory = os.path.abspath("cache")
cache_directory = os.path.abspath("executorlib_cache")
os.makedirs(cache_directory, exist_ok=True)
task_key, data_dict = serialize_funct_h5(
fn=get_error,
Expand Down Expand Up @@ -142,5 +142,4 @@ def test_execute_function_error(self):
future_file_obj.result()

def tearDown(self):
if os.path.exists("cache"):
shutil.rmtree("cache")
shutil.rmtree("executorlib_cache", ignore_errors=True)
3 changes: 1 addition & 2 deletions tests/test_cache_fileexecutor_mpi.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,5 +40,4 @@ def test_executor(self):
self.assertTrue(fs1.done())

def tearDown(self):
if os.path.exists("cache"):
shutil.rmtree("cache")
shutil.rmtree("executorlib_cache", ignore_errors=True)
9 changes: 4 additions & 5 deletions tests/test_cache_fileexecutor_serial.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ def test_executor_function(self):
"resource_dict": {},
}
)
cache_dir = os.path.abspath("cache")
cache_dir = os.path.abspath("executorlib_cache")
os.makedirs(cache_dir, exist_ok=True)
process = Thread(
target=execute_tasks_h5,
Expand Down Expand Up @@ -134,7 +134,7 @@ def test_executor_function_dependence_kwargs(self):
"resource_dict": {},
}
)
cache_dir = os.path.abspath("cache")
cache_dir = os.path.abspath("executorlib_cache")
os.makedirs(cache_dir, exist_ok=True)
process = Thread(
target=execute_tasks_h5,
Expand Down Expand Up @@ -175,7 +175,7 @@ def test_executor_function_dependence_args(self):
"resource_dict": {},
}
)
cache_dir = os.path.abspath("cache")
cache_dir = os.path.abspath("executorlib_cache")
os.makedirs(cache_dir, exist_ok=True)
process = Thread(
target=execute_tasks_h5,
Expand Down Expand Up @@ -203,5 +203,4 @@ def test_execute_in_subprocess_errors(self):
execute_in_subprocess(file_name=__file__, command=[], backend="flux")

def tearDown(self):
if os.path.exists("cache"):
shutil.rmtree("cache")
shutil.rmtree("executorlib_cache", ignore_errors=True)
7 changes: 3 additions & 4 deletions tests/test_fluxclusterexecutor.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@ def mpi_funct(i):
class TestCacheExecutorPysqa(unittest.TestCase):
def test_executor(self):
with FluxClusterExecutor(
resource_dict={"cores": 2, "cwd": "cache"},
resource_dict={"cores": 2, "cwd": "executorlib_cache"},
block_allocation=False,
cache_directory="cache",
cache_directory="executorlib_cache",
) as exe:
cloudpickle_register(ind=1)
fs1 = exe.submit(mpi_funct, 1)
Expand All @@ -44,5 +44,4 @@ def test_executor(self):
self.assertTrue(fs1.done())

def tearDown(self):
if os.path.exists("cache"):
shutil.rmtree("cache")
shutil.rmtree("executorlib_cache", ignore_errors=True)
6 changes: 3 additions & 3 deletions tests/test_mpiexecspawner.py
Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,7 @@ def test_execute_task_parallel(self):

class TestFuturePoolCache(unittest.TestCase):
def tearDown(self):
shutil.rmtree("./cache")
shutil.rmtree("executorlib_cache", ignore_errors=True)

@unittest.skipIf(
skip_h5py_test, "h5py is not installed, so the h5py tests are skipped."
Expand All @@ -519,7 +519,7 @@ def test_execute_task_cache(self):
cores=1,
openmpi_oversubscribe=False,
spawner=MpiExecSpawner,
cache_directory="./cache",
cache_directory="executorlib_cache",
)
self.assertEqual(f.result(), 1)
q.join()
Expand All @@ -538,6 +538,6 @@ def test_execute_task_cache_failed_no_argument(self):
cores=1,
openmpi_oversubscribe=False,
spawner=MpiExecSpawner,
cache_directory="./cache",
cache_directory="executorlib_cache",
)
q.join()
10 changes: 4 additions & 6 deletions tests/test_singlenodeexecutor_cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ def get_error(a):
)
class TestCacheFunctions(unittest.TestCase):
def test_cache_data(self):
cache_directory = "./cache"
cache_directory = os.path.abspath("executorlib_cache")
with SingleNodeExecutor(cache_directory=cache_directory) as exe:
self.assertTrue(exe)
future_lst = [exe.submit(sum, [i, i]) for i in range(1, 4)]
Expand All @@ -35,7 +35,7 @@ def test_cache_data(self):
)

def test_cache_error(self):
cache_directory = "./cache_error"
cache_directory = os.path.abspath("cache_error")
with SingleNodeExecutor(cache_directory=cache_directory) as exe:
self.assertTrue(exe)
cloudpickle_register(ind=1)
Expand All @@ -44,7 +44,5 @@ def test_cache_error(self):
print(f.result())

def tearDown(self):
if os.path.exists("cache"):
shutil.rmtree("cache")
if os.path.exists("cache_error"):
shutil.rmtree("cache_error")
shutil.rmtree("executorlib_cache", ignore_errors=True)
shutil.rmtree("cache_error", ignore_errors=True)
4 changes: 2 additions & 2 deletions tests/test_singlenodeexecutor_mpi.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ def test_errors(self):

class TestExecutorBackendCache(unittest.TestCase):
def tearDown(self):
shutil.rmtree("./cache")
shutil.rmtree("executorlib_cache", ignore_errors=True)

@unittest.skipIf(
skip_mpi4py_test, "mpi4py is not installed, so the mpi4py tests are skipped."
Expand All @@ -93,7 +93,7 @@ def test_meta_executor_parallel_cache(self):
max_workers=2,
resource_dict={"cores": 2},
block_allocation=True,
cache_directory="./cache",
cache_directory="executorlib_cache",
) as exe:
cloudpickle_register(ind=1)
time_1 = time.time()
Expand Down
Loading
Loading