diff --git a/.gitignore b/.gitignore index 2b5d9c6a..82f217a9 100644 --- a/.gitignore +++ b/.gitignore @@ -44,6 +44,7 @@ wheels/ tests/resources/*/output_reports tests/resources/*/output_kdic tests/resources/dictionary/copy_output_kdic +tests/resources/scenario_generation/system_settings/output tests/resources/scenario_generation/api/*/output tests/resources/scenario_generation/data_path_deprecation/*/output tests/resources/scenario_generation/general_options/output diff --git a/doc/site/notes.md b/doc/site/notes.md index 533009ae..159c65f2 100644 --- a/doc/site/notes.md +++ b/doc/site/notes.md @@ -61,6 +61,33 @@ The functions in the [khiops.core.api][] have the following common parameters. : If `True` the internal scenario generated by Khiops will force characters such as accentuated ones to be decoded with the UTF8->ANSI khiops transformation. +#### Process-wide System-parameter Defaults { #core-api-global-system-params } + +The Core API provides process-wide defaults for subsequent local Khiops +executions. Each setter rebuilds the local runner with an isolated copy of the +current environment; it never changes the caller's complete `os.environ` +mapping. Existing per-call system parameters continue to apply to the generated +execution scenario. + +- [set_default_max_cores][khiops.core.api.set_default_max_cores] and + [get_default_max_cores][khiops.core.api.get_default_max_cores] configure and query + the maximum number of CPU cores. +- [set_default_memory_limit_mb][khiops.core.api.set_default_memory_limit_mb] and + [get_default_memory_limit_mb][khiops.core.api.get_default_memory_limit_mb] + configure and query the maximum amount of memory in megabytes. +- [set_default_temp_dir][khiops.core.api.set_default_temp_dir] and + [get_default_temp_dir][khiops.core.api.get_default_temp_dir] configure and query + the Khiops temporary directory. + +The numeric setters accept an optional positive `int`. The temporary-directory +setter accepts an optional non-empty `str`; invalid types and values raise +`TypeError` or `ValueError`, and a rejected update leaves the previous value +unchanged. Calling a setter with no arguments disables only that default and restores +the corresponding value from the environment captured when the Khiops package +initialized, or removes the variable if it was absent. Later changes to +`os.environ` do not change this reset target. Getters return typed numeric +values, a non-empty path, or `None` when the effective variable is absent. + ### Input Types { #core-api-input-types } The types accepted in most methods and classes of [khiops.core][] are flexible: diff --git a/khiops/core/api.py b/khiops/core/api.py index 3774c4c0..2c51feda 100644 --- a/khiops/core/api.py +++ b/khiops/core/api.py @@ -17,7 +17,9 @@ """ import io import os +import threading import warnings +from types import MappingProxyType import khiops.core.internals.filesystems as fs from khiops.core.dictionary import DictionaryDomain @@ -29,9 +31,176 @@ is_string_like, type_error_message, ) -from khiops.core.internals.runner import get_runner +from khiops.core.internals.runner import KhiopsLocalRunner, get_runner, set_runner from khiops.core.internals.task import get_task_registry +# Capture the process environment before any runner initialization can occur. +# Avoid race condition on os.environ (via MappingProxyType) and make sure updates +# to it are not propagated to the current environment (via .copy()). +_INHERITED_ENVIRONMENT = MappingProxyType(os.environ.copy()) +_CURRENT_ENVIRONMENT = dict(_INHERITED_ENVIRONMENT) +_DEFAULT_SYSTEM_PARAMETERS_LOCK = threading.RLock() + + +def _validate_default_system_parameter(parameter_name, value, expected_type): + """Validate a global system-parameter value.""" + if value is None: + return + + if not isinstance(value, expected_type) or ( + expected_type is int and isinstance(value, bool) + ): + raise TypeError(type_error_message(parameter_name, value, expected_type)) + if not value: + raise ValueError(f"{parameter_name} must be non-empty") + if isinstance(value, int) and value < 0: + raise ValueError(f"{parameter_name} must be positive; it is {value}") + + +def _set_default_system_parameter( + parameter_name, environment_variable, value, expected_type +): + """Set one global system parameter and install its runner.""" + global _CURRENT_ENVIRONMENT + + _validate_default_system_parameter(parameter_name, value, expected_type) + with _DEFAULT_SYSTEM_PARAMETERS_LOCK: + updated_environment = _CURRENT_ENVIRONMENT.copy() + if value is None: + inherited_value = _INHERITED_ENVIRONMENT.get(environment_variable) + if inherited_value is None and environment_variable in updated_environment: + del updated_environment[environment_variable] + elif inherited_value is not None: + updated_environment[environment_variable] = inherited_value + else: + updated_environment[environment_variable] = str(value) + + runner = KhiopsLocalRunner(environment=updated_environment.copy()) + _CURRENT_ENVIRONMENT = updated_environment + set_runner(runner) + + +def _get_default_system_parameter(environment_variable, expected_type): + """Get one global system parameter from the current environment.""" + with _DEFAULT_SYSTEM_PARAMETERS_LOCK: + value = _CURRENT_ENVIRONMENT.get(environment_variable) + if value is None: + return None + if expected_type is int: + return int(value) + return value + + +def set_default_max_cores(max_cores=None): + """Set the process-wide default maximum number of Khiops cores. + + Parameters + ---------- + max_cores : int, optional + Positive maximum number of cores. If not specified, restore the value + captured when the Khiops package initialized. + + Notes + ----- + The value is applied to subsequent local executions through an isolated + runner environment and does not modify `os.environ`. + + Raises + ------ + TypeError + If `max_cores` is not an `int` or `None`. + ValueError + If `max_cores` is not positive. + """ + _set_default_system_parameter("max_cores", "KHIOPS_PROC_NUMBER", max_cores, int) + + +def get_default_max_cores(): + """Return the process-wide default maximum number of Khiops cores. + + Returns + ------- + int or None + The effective positive maximum number of cores, or `None` when no + default is configured. + """ + return _get_default_system_parameter("KHIOPS_PROC_NUMBER", int) + + +def set_default_memory_limit_mb(memory_limit_mb=None): + """Set the process-wide default Khiops memory limit. + + Parameters + ---------- + memory_limit_mb : int, optional + Positive memory limit in megabytes. If not specified, restore the value + captured when the Khiops package initialized. + + Notes + ----- + The value is applied to subsequent local executions through an isolated + runner environment and does not modify `os.environ`. + + Raises + ------ + TypeError + If `memory_limit_mb` is not an `int` or `None`. + ValueError + If `memory_limit_mb` is not positive. + """ + _set_default_system_parameter( + "memory_limit_mb", "KHIOPS_MEMORY_LIMIT", memory_limit_mb, int + ) + + +def get_default_memory_limit_mb(): + """Return the process-wide default Khiops memory limit. + + Returns + ------- + int or None + The effective positive memory limit in megabytes, or `None` when no + default is configured. + """ + return _get_default_system_parameter("KHIOPS_MEMORY_LIMIT", int) + + +def set_default_temp_dir(temp_dir=None): + """Set the process-wide default temporary directory for Khiops. + + Parameters + ---------- + temp_dir : str, optional + Non-empty temporary-directory path. If not specified, restore the value + captured when the Khiops package initialized. + + Notes + ----- + The value is applied to subsequent local executions through an isolated + runner environment and does not modify `os.environ`. + + Raises + ------ + TypeError + If `temp_dir` is not a `str` or `None`. + ValueError + If `temp_dir` is empty. + """ + _set_default_system_parameter("temp_dir", "KHIOPS_TMP_DIR", temp_dir, str) + + +def get_default_temp_dir(): + """Return the process-wide default temporary directory for Khiops. + + Returns + ------- + str or None + The effective non-empty temporary-directory path, or `None` when no + default is configured. + """ + return _get_default_system_parameter("KHIOPS_TMP_DIR", str) + + # Construction rules DEFAULT_CONSTRUCTION_RULES = [ "GetValue", diff --git a/khiops/core/internals/runner.py b/khiops/core/internals/runner.py index e210a3c5..e6553366 100644 --- a/khiops/core/internals/runner.py +++ b/khiops/core/internals/runner.py @@ -50,7 +50,14 @@ def _isdir_without_all_perms(dir_path): ) -def get_default_samples_dir(): +def _current_environment(environment=None): + """Returns the provided environment or the process environment.""" + if environment is not None: + return environment + return os.environ + + +def get_default_samples_dir(environment=None): """Returns the default samples directory The default samples directory is computed according to the following priorities: @@ -60,16 +67,20 @@ def get_default_samples_dir(): - `%USERPROFILE%\\khiops_data\\samples` otherwise - Linux/macOS: `$HOME/khiops_data/samples` """ - if "KHIOPS_SAMPLES_DIR" in os.environ and os.environ["KHIOPS_SAMPLES_DIR"]: - samples_dir = os.environ["KHIOPS_SAMPLES_DIR"] - elif platform.system() == "Windows" and "PUBLIC" in os.environ: - samples_dir = os.path.join(os.environ["PUBLIC"], "khiops_data", "samples") + environment = _current_environment(environment) + if "KHIOPS_SAMPLES_DIR" in environment and environment["KHIOPS_SAMPLES_DIR"]: + samples_dir = environment["KHIOPS_SAMPLES_DIR"] + elif platform.system() == "Windows" and "PUBLIC" in environment: + samples_dir = os.path.join(environment["PUBLIC"], "khiops_data", "samples") else: # The filesystem abstract layer is used here # as the path can be either local or remote - samples_dir = fs.get_child_path( - fs.get_child_path(os.environ["HOME"], "khiops_data"), "samples" - ) + if "HOME" in environment: + samples_dir = fs.get_child_path( + fs.get_child_path(environment["HOME"], "khiops_data"), "samples" + ) + else: + raise KeyError("HOME") return samples_dir @@ -237,7 +248,7 @@ def _check_conda_env_bin_dir(conda_env_bin_dir): return is_conda_env_bin_dir -def _infer_khiops_installation_method(trace=False): +def _infer_khiops_installation_method(environment=None, trace=False): """Returns the Khiops installation method Definitions : @@ -255,6 +266,8 @@ def _infer_khiops_installation_method(trace=False): - or in a classical virtual environment (highly encouraged) """ + environment = _current_environment(environment) + # We are in a Conda environment if # - the CONDA_PREFIX environment variable exists and, # - the khiops_env script exists within: @@ -263,8 +276,8 @@ def _infer_khiops_installation_method(trace=False): # Note: The check that the Khiops binaries are actually executable is done # afterwards by the initializations method. installation_method = "unknown" - if "CONDA_PREFIX" in os.environ: - conda_env_dir = os.environ["CONDA_PREFIX"] + if "CONDA_PREFIX" in environment: + conda_env_dir = environment["CONDA_PREFIX"] if platform.system() == "Windows": conda_binary_dir = os.path.join(conda_env_dir, "Library", "bin") else: @@ -335,13 +348,16 @@ def _get_current_library_installer(): return "unknown" -def _build_khiops_process_environment(): +def _build_khiops_process_environment(environment=None): """Build a specific environment used for the execution of khiops in a process This environment can be modified freely without interfering with the global one. """ - khiops_env = os.environ.copy() + if environment is not None: + khiops_env = environment.copy() + else: + khiops_env = os.environ.copy() # Ensure HOME is always set for OpenMPI 5+ # (using KHIOPS_MPI_HOME if it exists) @@ -946,8 +962,17 @@ class KhiopsLocalRunner(KhiopsRunner): """ - def __init__(self): + def __init__(self, environment=None): + """Initialize a local runner. + + Parameters + ---------- + environment : dict, optional + Environment owned by this runner. If omitted, initialization keeps + using the process environment for compatibility. + """ # Define specific attributes + self._environment = environment.copy() if environment is not None else None self._mpi_command_args = None self._khiops_path = None self._khiops_coclustering_path = None @@ -962,7 +987,10 @@ def __init__(self): self._initialize_khiops_environment() def _initialize_khiops_environment(self): - installation_method = _infer_khiops_installation_method() + runner_environment = _current_environment(self._environment) + installation_method = _infer_khiops_installation_method( + environment=runner_environment + ) match installation_method: # In conda-based environments, khiops_env is not in PATH; # its location must be inferred from the conda env directory. @@ -974,7 +1002,9 @@ def _initialize_khiops_environment(self): khiops_env_path += ".cmd" # In an activated conda environment, khiops_env is in PATH. case "conda": - khiops_env_path = self._infer_khiops_env_from_path(installation_method) + khiops_env_path = self._infer_khiops_env_from_path( + installation_method, runner_environment + ) case "pip": # Ensure the binary dependency is still installed. try: @@ -1011,18 +1041,23 @@ def _initialize_khiops_environment(self): # which is in PATH. else: khiops_env_path = self._infer_khiops_env_from_path( - installation_method + installation_method, runner_environment ) case _: raise KhiopsEnvironmentError( f"Unknown installation method '{installation_method}'." ) + khiops_env_process_options = { + "stdout": subprocess.PIPE, + "stderr": subprocess.PIPE, + "universal_newlines": True, + } + if self._environment is not None: + khiops_env_process_options["env"] = runner_environment.copy() + with subprocess.Popen( - [khiops_env_path, "--env"], - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - universal_newlines=True, + [khiops_env_path, "--env"], **khiops_env_process_options ) as khiops_env_process: stdout, stderr = khiops_env_process.communicate() if khiops_env_process.returncode != 0: @@ -1054,32 +1089,33 @@ def _initialize_khiops_environment(self): # to prepend to or to set to HOME for OpenMPI 5+ # when running khiops core if var_name == "KHIOPS_MPI_HOME": - os.environ["KHIOPS_MPI_HOME"] = var_value + runner_environment["KHIOPS_MPI_HOME"] = var_value # Set paths to Khiops binaries elif var_name == "KHIOPS_PATH": self.khiops_path = var_value - os.environ["KHIOPS_PATH"] = var_value + runner_environment["KHIOPS_PATH"] = var_value elif var_name == "KHIOPS_COCLUSTERING_PATH": self.khiops_coclustering_path = var_value - os.environ["KHIOPS_COCLUSTERING_PATH"] = var_value + runner_environment["KHIOPS_COCLUSTERING_PATH"] = var_value # Set MPI command elif var_name == "KHIOPS_MPI_COMMAND": self._mpi_command_args = shlex.split(var_value) - os.environ["KHIOPS_MPI_COMMAND"] = var_value + runner_environment["KHIOPS_MPI_COMMAND"] = var_value # On Windows, in 'pip' installations, # "KHIOPS_MPI_DLL_PATH" (containing the Intel MPI Library) # must be added to "PATH" otherwise Khiops wouldn't find it # and fail immediately elif installation_method == "pip" and var_name == "KHIOPS_MPI_DLL_PATH": - os.environ["PATH"] = os.pathsep.join( - [var_value, os.environ.get("PATH")] + runner_environment["PATH"] = os.pathsep.join( + [var_value, runner_environment.get("PATH")] ) + runner_environment["KHIOPS_MPI_DLL_PATH"] = var_value # Propagate all the other environment variables to Khiops binaries else: - os.environ[var_name] = var_value + runner_environment[var_name] = var_value - # Set KHIOPS_API_MODE to `true` - os.environ["KHIOPS_API_MODE"] = "true" + # Set KHIOPS_API_MODE to `true` + runner_environment["KHIOPS_API_MODE"] = "true" # Check the tools exist and are executable self._check_tools() @@ -1087,8 +1123,11 @@ def _initialize_khiops_environment(self): # Initialize the default samples dir self._initialize_default_samples_dir() - def _infer_khiops_env_from_path(self, installation_method): - khiops_env_path = shutil.which("khiops_env") + def _infer_khiops_env_from_path(self, installation_method, environment=None): + # Fall back to searching `khiops_env` according to the `PATH` set in + # `os.environ`. + search_path = environment.get("PATH") if environment is not None else None + khiops_env_path = shutil.which("khiops_env", path=search_path) if khiops_env_path is None: raise KhiopsEnvironmentError( "The 'khiops_env' script not found for the current " @@ -1100,7 +1139,7 @@ def _infer_khiops_env_from_path(self, installation_method): def _initialize_default_samples_dir(self): """See class docstring""" - samples_dir = get_default_samples_dir() + samples_dir = get_default_samples_dir(self._environment) _check_samples_dir(samples_dir) self._samples_dir = samples_dir assert self._samples_dir is not None @@ -1162,7 +1201,10 @@ def _detect_library_installation_incompatibilities(self, library_root_dir_path): error_list = [] warning_list = [] - installation_method = _infer_khiops_installation_method() + runner_environment = _current_environment(self._environment) + installation_method = _infer_khiops_installation_method( + environment=runner_environment + ) # activated 'conda' installation if installation_method == "conda": @@ -1182,14 +1224,14 @@ def _detect_library_installation_incompatibilities(self, library_root_dir_path): ) warning_list.append(warning) - conda_prefix_path = Path(os.environ["CONDA_PREFIX"]) + conda_prefix_path = Path(runner_environment["CONDA_PREFIX"]) # the conda environment must match the library installation if not library_root_dir_path.is_relative_to(conda_prefix_path): error = ( "Khiops Python library installation " f"path '{library_root_dir_path}' " "does not match the current Conda environment " - f"'{os.environ['CONDA_PREFIX']}'. " + f"'{runner_environment['CONDA_PREFIX']}'. " "Either deactivate the current Conda environment " "or use the Khiops Python library " "belonging to the current Conda environment. " @@ -1203,7 +1245,7 @@ def _detect_library_installation_incompatibilities(self, library_root_dir_path): error = ( f"Khiops binary path '{self.khiops_path}' " "does not match the current Conda environment " - f"'{os.environ['CONDA_PREFIX']}'. " + f"'{runner_environment['CONDA_PREFIX']}'. " "We recommend installing the Khiops binary " "in the current Conda environment. " "Go to https://khiops.org for instructions.\n" @@ -1335,7 +1377,10 @@ def _build_status_message(self): ) # Build the messages for install type and mpi - install_type_msg = _infer_khiops_installation_method() + runner_environment = _current_environment(self._environment) + install_type_msg = _infer_khiops_installation_method( + environment=runner_environment + ) if self._mpi_command_args: mpi_command_args_msg = " ".join(self._mpi_command_args) else: @@ -1523,7 +1568,7 @@ def raw_run(self, tool_name, command_line_args=None, use_mpi=True, trace=False): # Build custom Khiops process environment # which makes sure HOME is defined and set # according to khiops_env's KHIOPS_MPI_HOME - khiops_env = _build_khiops_process_environment() + khiops_env = _build_khiops_process_environment(self._environment) # Execute the process with subprocess.Popen( diff --git a/tests/test_core.py b/tests/test_core.py index facdc667..2ed1e664 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -19,12 +19,13 @@ from copy import copy from difflib import unified_diff from pathlib import Path +from types import MappingProxyType from unittest import mock import khiops import khiops.core as kh import khiops.core.internals.filesystems as fs -from khiops.core import KhiopsRuntimeError +from khiops.core import KhiopsRuntimeError, api from khiops.core.api import _deprecate_legacy_data_path from khiops.core.internals.io import KhiopsOutputWriter from khiops.core.internals.runner import KhiopsLocalRunner, KhiopsRunner @@ -1020,6 +1021,177 @@ def mocked_raw_run(*_, **__): return mocked_raw_run +class KhiopsGlobalSystemParameterTests(unittest.TestCase): + """Test process-wide Core API system-parameter defaults""" + + def setUp(self): + self._initial_inherited_environment = api._INHERITED_ENVIRONMENT + self._initial_current_environment = api._CURRENT_ENVIRONMENT + + launch_environment = os.environ.copy() + launch_environment.update( + { + "KHIOPS_PROC_NUMBER": "2", + "KHIOPS_MEMORY_LIMIT": "128", + "KHIOPS_TMP_DIR": "/launch/tmp", + } + ) + api._INHERITED_ENVIRONMENT = MappingProxyType(launch_environment) + api._CURRENT_ENVIRONMENT = launch_environment.copy() + + self._runner_constructor_patch = mock.patch.object(api, "KhiopsLocalRunner") + self.runner_constructor = self._runner_constructor_patch.start() + self._set_runner_patch = mock.patch.object(api, "set_runner") + self.set_runner = self._set_runner_patch.start() + + def tearDown(self): + self._set_runner_patch.stop() + self._runner_constructor_patch.stop() + api._INHERITED_ENVIRONMENT = self._initial_inherited_environment + api._CURRENT_ENVIRONMENT = self._initial_current_environment + + def test_accessors_are_public_and_restore_launch_values(self): + """Test accessors expose typed defaults and inherited reset values""" + accessor_names = [ + "set_default_max_cores", + "get_default_max_cores", + "set_default_memory_limit_mb", + "get_default_memory_limit_mb", + "set_default_temp_dir", + "get_default_temp_dir", + ] + for accessor_name in accessor_names: + with self.subTest(accessor_name=accessor_name): + self.assertTrue(hasattr(kh, accessor_name)) + + self.assertEqual(kh.get_default_max_cores(), 2) + self.assertEqual(kh.get_default_memory_limit_mb(), 128) + self.assertEqual(kh.get_default_temp_dir(), "/launch/tmp") + + kh.set_default_max_cores(8) + kh.set_default_memory_limit_mb(512) + kh.set_default_temp_dir("/configured/tmp") + self.assertEqual(kh.get_default_max_cores(), 8) + self.assertEqual(kh.get_default_memory_limit_mb(), 512) + self.assertEqual(kh.get_default_temp_dir(), "/configured/tmp") + + kh.set_default_max_cores() + self.assertEqual(kh.get_default_max_cores(), 2) + self.assertEqual(kh.get_default_memory_limit_mb(), 512) + self.assertEqual(kh.get_default_temp_dir(), "/configured/tmp") + + def test_defaults_use_isolated_complete_runner_environment(self): + """Test accepted updates preserve the process environment""" + initial_process_environment = os.environ.copy() + + kh.set_default_max_cores(8) + + self.assertEqual(os.environ, initial_process_environment) + self.assertEqual(self.runner_constructor.call_count, 1) + runner_environment = self.runner_constructor.call_args.kwargs["environment"] + self.assertEqual(runner_environment["KHIOPS_PROC_NUMBER"], "8") + self.assertEqual(runner_environment["KHIOPS_MEMORY_LIMIT"], "128") + self.assertEqual(runner_environment["KHIOPS_TMP_DIR"], "/launch/tmp") + self.assertEqual(set(runner_environment), set(api._CURRENT_ENVIRONMENT)) + self.assertIsNot(runner_environment, api._CURRENT_ENVIRONMENT) + + def test_later_environment_changes_do_not_change_reset_targets(self): + """Test reset values remain tied to the launch snapshot""" + with mock.patch.dict(os.environ, {"KHIOPS_PROC_NUMBER": "99"}): + kh.set_default_max_cores(8) + kh.set_default_max_cores(None) + + self.assertEqual(kh.get_default_max_cores(), 2) + + def test_absent_inherited_values_are_removed_on_reset(self): + """Test resetting an absent inherited value removes the variable""" + launch_environment = dict(api._INHERITED_ENVIRONMENT) + for environment_variable in ( + "KHIOPS_PROC_NUMBER", + "KHIOPS_MEMORY_LIMIT", + "KHIOPS_TMP_DIR", + ): + launch_environment.pop(environment_variable) + api._INHERITED_ENVIRONMENT = MappingProxyType(launch_environment) + api._CURRENT_ENVIRONMENT = launch_environment.copy() + + kh.set_default_max_cores(8) + kh.set_default_memory_limit_mb(512) + kh.set_default_temp_dir("/configured/tmp") + kh.set_default_max_cores() + kh.set_default_memory_limit_mb() + kh.set_default_temp_dir() + + self.assertIsNone(kh.get_default_max_cores()) + self.assertIsNone(kh.get_default_memory_limit_mb()) + self.assertIsNone(kh.get_default_temp_dir()) + runner_environment = self.runner_constructor.call_args.kwargs["environment"] + self.assertNotIn("KHIOPS_PROC_NUMBER", runner_environment) + self.assertNotIn("KHIOPS_MEMORY_LIMIT", runner_environment) + self.assertNotIn("KHIOPS_TMP_DIR", runner_environment) + + def test_invalid_updates_leave_values_unchanged(self): + """Test invalid values are rejected without changing effective defaults""" + invalid_updates = [ + ( + kh.set_default_max_cores, + 0, + kh.get_default_max_cores, + "max_cores must be non-empty", + ), + ( + kh.set_default_max_cores, + -1, + kh.get_default_max_cores, + "max_cores must be positive; it is -1", + ), + ( + kh.set_default_max_cores, + "8", + kh.get_default_max_cores, + "'max_cores' type must be 'int', not 'str'", + ), + ( + kh.set_default_memory_limit_mb, + 0, + kh.get_default_memory_limit_mb, + "memory_limit_mb must be non-empty", + ), + ( + kh.set_default_memory_limit_mb, + "512", + kh.get_default_memory_limit_mb, + "'memory_limit_mb' type must be 'int', not 'str'", + ), + ( + kh.set_default_temp_dir, + "", + kh.get_default_temp_dir, + "temp_dir must be non-empty", + ), + ( + kh.set_default_temp_dir, + 42, + kh.get_default_temp_dir, + "'temp_dir' type must be 'str', not 'int'", + ), + ] + + for setter, invalid_value, getter, expected_message in invalid_updates: + with self.subTest(invalid_value=invalid_value): + with self.assertRaises((TypeError, ValueError)) as context: + setter(invalid_value) + self.assertEqual(str(context.exception), expected_message) + self.assertEqual( + getter(), + { + kh.get_default_max_cores: 2, + kh.get_default_memory_limit_mb: 128, + kh.get_default_temp_dir: "/launch/tmp", + }[getter], + ) + + class KhiopsCoreServicesTests(unittest.TestCase): """Test the services of the core module classes diff --git a/tests/test_khiops_integrations.py b/tests/test_khiops_integrations.py index ee3132db..e7b14a03 100644 --- a/tests/test_khiops_integrations.py +++ b/tests/test_khiops_integrations.py @@ -17,6 +17,7 @@ import khiops.core as kh import khiops.core.internals.filesystems as fs +import khiops.core.internals.runner as runner_module from khiops import tools from khiops.core.exceptions import KhiopsEnvironmentError from khiops.core.internals.runner import ( @@ -346,6 +347,63 @@ def test_runner_environment_initialization(self): self.assertEqual(env_khiops_api_mode, "true") + def test_isolated_runner_environment_is_not_written_to_process(self): + """Test explicit runner environments reach both subprocess boundaries""" + initial_process_environment = os.environ.copy() + isolated_environment = initial_process_environment.copy() + isolated_environment.update( + { + "KHIOPS_PROC_NUMBER": "7", + "KHIOPS_MEMORY_LIMIT": "256", + "KHIOPS_TMP_DIR": "/isolated/tmp", + "HOME": "/isolated/home", + } + ) + tool_path = os.environ.get("SHELL", "/bin/sh") + + mock_popen = MagicMock() + mock_popen.return_value.__enter__.return_value.communicate.return_value = ( + "KHIOPS_MPI_HOME /isolated/mpi\n" + f"KHIOPS_PATH {tool_path}\n" + f"KHIOPS_COCLUSTERING_PATH {tool_path}\n" + "KHIOPS_MPI_COMMAND mpiexec\n" + "KHIOPS_API_MODE false\n" + "KHIOPS_RETURNED yes\n", + "", + ) + mock_popen.return_value.__enter__.return_value.returncode = 0 + + with patch.object( + runner_module, "_infer_khiops_installation_method", return_value="conda" + ): + with patch.object( + KhiopsLocalRunner, + "_infer_khiops_env_from_path", + return_value="/bin/echo", + ): + with patch.object(KhiopsLocalRunner, "_check_tools"): + with patch.object( + KhiopsLocalRunner, "_initialize_default_samples_dir" + ): + with patch.object( + runner_module.subprocess, "Popen", mock_popen + ): + runner = KhiopsLocalRunner(environment=isolated_environment) + runner.raw_run("khiops", [], use_mpi=False) + + self.assertEqual(os.environ, initial_process_environment) + self.assertEqual(runner._environment["KHIOPS_RETURNED"], "yes") + self.assertEqual(runner._environment["KHIOPS_API_MODE"], "true") + self.assertEqual( + mock_popen.call_args_list[0].kwargs["env"], isolated_environment + ) + final_environment = mock_popen.call_args_list[1].kwargs["env"] + self.assertEqual(final_environment["KHIOPS_PROC_NUMBER"], "7") + self.assertEqual(final_environment["KHIOPS_MEMORY_LIMIT"], "256") + self.assertEqual(final_environment["KHIOPS_TMP_DIR"], "/isolated/tmp") + self.assertEqual(final_environment["KHIOPS_RETURNED"], "yes") + self.assertEqual(final_environment["HOME"], "/isolated/home") + def test_khiops_and_khiops_coclustering_are_run_with_mpi(self): """Test that MODL and MODL_Coclustering are run with MPI"""