Skip to content

Set the KHIOPS_PROC_NUMBER env var for each training run using the value of max_cores - #623

Open
tramora wants to merge 1 commit into
mainfrom
max-cores-allocated
Open

tramora wants to merge 1 commit into
mainfrom
max-cores-allocated

Conversation

@tramora

@tramora tramora commented Sep 20, 2026 •

Copy link
Copy Markdown
Collaborator
  • the allocated number of CPU cores can never be greater than the value of max_cores

Fixes #585


TODO Before Asking for a Review

  • Rebase your branch to the latest version of main (or main-v10)
  • Make sure all CI workflows are green
  • When adding a public feature/fix: Update the Unreleased section of CHANGELOG.md (no date)
  • Self-Review: Review "Files Changed" tab and fix any problems you find
  • API Docs (only if there are changes in docstrings, markdown files or samples):
    • Check the docs build without warning: see the log of the API Docs workflow
    • Check that your changes render well in HTML: download the API Docs artifact and open index.html
    • If there are any problems it is faster to iterate by building locally the API Docs

…value of `max_cores`

- the allocated number of CPU cores can never be greater than the value of `max_cores`
@tramora
tramora force-pushed the max-cores-allocated branch from b2f355a to e532ce2 Compare September 22, 2026 09:53
@tramora
tramora requested a review from popescu-v September 22, 2026 09:54
# An additional environment variable (local to this specific run)
# must also be set to avoid allocating all the available CPU cores.
# Thus, allocated CPU cores = max number of CPU cores used
khiops_env["KHIOPS_PROC_NUMBER"] = system_settings.max_cores

@popescu-v popescu-v Sep 24, 2026 •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is too late: KHIOPS_PROC_NUMBER needs to be set in a specific environment in which khiops_env --env is launched, because it is therein that KHIOPS_MPI_COMMAND is computed with respect to KHIOPS_PROC_NUMBER (basically, -n $KHIOPS_PROC_NUMBER is added to mpiexec). Otherwise, khiops_process_args still contains the mpiexec command without the -n option set according to KHIOPS_PROC_NUMBER: indeed, the MPI command is given by KhiopsLocalRunner._mpi_command_args, which is computed when khiops_env is read.

Hence, IMHO the way forward is to build and propagate a custom environment (a copy of os.environ, with a KHIOPS_PROC_NUMBER set according to max_cores) to the khiops_env --env subprocess. To this end, I would create a new KhiopsLocalRunner instance and set via set_runner, within the _run_task method. For this to work on custom environments, IMHO we need to add an environment parameter to KhiopsLocalRunner (cf. the changes in https://github.com/KhiopsML/khiops-python/pull/624/changes#diff-d3a8e2b402a205257210c09d4cdb06e74ddc307bb2b5b9079467a9c79934bce2).

if initial_home:
os.environ["HOME"] = initial_home

def test_runner_environment_max_cores_sets_proc_number_env(self):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The relevant test is to:

  1. set max_cores on an API function;
  2. call the function and capture its local runner's state;
  3. on this state, check that ._mpi_command_args contains -n ...according tomax_cores`.

For 2., use the wrapt-based aproach (cf. test_sklearn.py, where the wrap_with_parameter_trace is used). On first thought, I'd go for defining a wrap_with_runner_trace helper, whereby I'd trace the runner used when running the API function, by:

  • wrapping the KhiopsRunner.run method inside the wrap_with_runner_trace to trace self (self being the KhiopsLocalRunner instance).
  • then, on that self trace, I'd check self._mpi_command_args for the existence of the ["-n", max_cores] sublist.

@popescu-v popescu-v left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

See the comments.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use KHIOPS_PROC_NUMBER when max_cores is set

2 participants