fzd: parallelize Python-function model evaluations when calculators>1 - #80
Merged
Conversation
…tors>1 fzd(..., model=<callable>, calculators=N) previously accepted N only for API compatibility and always ran evaluations sequentially in the calling thread, regardless of its value -- required because a Python-side thread pool always dispatches to a worker thread (even with a single worker), which crashes callables that must be invoked from the thread that created them (e.g. an R closure bridged in via reticulate). calculators=1 (the default) keeps that exact behavior: strictly sequential, one call at a time, in the calling thread, no thread pool at all. calculators=N>1 now dispatches evaluations to a concurrent.futures.ThreadPoolExecutor with N worker threads, running them concurrently -- intended for an ordinary, thread-safe Python function. If any point raises while evaluated in parallel, the whole batch aborts immediately with an explicit FunctionModelParallelError (a RuntimeError subclass) naming the offending point, the calculators value, and suggesting a retry with calculators=1 -- instead of being silently downgraded to a per-point failure as sequential mode does for ordinary model errors. A thread-safety problem can otherwise surface as sporadic, hard to diagnose per-point failures, so failing the whole run loudly is safer than continuing. Also validates that calculators is a positive int in this mode (raises ValueError for calculators <= 0), and exports FunctionModelParallelError from the top-level fz package. Companion change: Funz/fz.R must keep forcing calculators=1 for R-function models now that calculators>1 has a real effect on this side (R closures bridged in via reticulate are not safe to call from a worker thread). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates fz.fzd(..., model=<callable>, calculators=N) so that Python-function model evaluations can actually run in parallel when calculators > 1, while preserving the safe default behavior (calculators=1 runs sequentially in the calling thread). It also introduces a dedicated exception type to make parallel-evaluation failures explicit and fatal for the whole batch.
Changes:
- Implement parallel execution for function-model designs via
ThreadPoolExecutorwhencalculators > 1, and keepcalculators <= 1strictly sequential in the calling thread. - Add and export
fz.FunctionModelParallelErrorto surface parallel evaluation failures as fatal (rather than per-point failures). - Add a new test module covering sequential semantics, parallel semantics, error propagation, ordering, and calculators validation.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
fz/core.py |
Implements parallel function-model evaluation, adds FunctionModelParallelError, tightens calculators validation, and updates fzd docstrings/behavior. |
fz/__init__.py |
Exports FunctionModelParallelError at the top-level fz package. |
tests/test_fzd_function_model_parallel.py |
Adds unit + end-to-end tests for function-model parallelization behavior and validation. |
Suppressed comments (1)
fz/core.py:2282
- The calculators validation treats bools as ints (since bool is a subclass of int), so callers can pass calculators=True/False and it will be accepted (True behaves like 1, False triggers the “< 1” ValueError). Given calculators is a user-facing API parameter, it’s better to reject bool explicitly to avoid surprising behavior.
if calculators is None:
function_workers = 1
elif isinstance(calculators, int):
if calculators < 1:
raise ValueError(
f"calculators must be a positive int when model is a Python callable, got {calculators}"
)
function_workers = calculators
else:
raise TypeError("calculators must be an int (number of parallel calls) when model is a Python callable")
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+2082
to
+2099
| results = [None] * len(design_points) | ||
| with ThreadPoolExecutor(max_workers=max_workers) as executor: | ||
| future_to_index = {executor.submit(_call, point): i for i, point in enumerate(design_points)} | ||
| for future in as_completed(future_to_index): | ||
| index = future_to_index[future] | ||
| try: | ||
| results[index] = future.result() | ||
| except Exception as e: | ||
| point = design_points[index] | ||
| raise FunctionModelParallelError( | ||
| f"Model function raised an error while evaluating point {point} " | ||
| f"during parallel execution (calculators={max_workers}): {e}. " | ||
| "If this function is not safe to call concurrently from multiple " | ||
| "threads (e.g. a callable bridged in from another language runtime, " | ||
| "such as an R function via reticulate), retry with calculators=1 to " | ||
| "force strictly sequential, single-threaded execution." | ||
| ) from e | ||
| return results |
Comment on lines
+70
to
+90
| def test_parallel_runs_across_multiple_threads(self): | ||
| seen_threads = set() | ||
| lock = threading.Lock() | ||
|
|
||
| def model_func(x): | ||
| with lock: | ||
| seen_threads.add(threading.current_thread().ident) | ||
| time.sleep(0.05) | ||
| return {"y": x * 2} | ||
|
|
||
| design_points = [{"x": i} for i in range(8)] | ||
| start = time.time() | ||
| results = _run_function_model_design(model_func, design_points, None, 4) | ||
| elapsed = time.time() - start | ||
|
|
||
| assert sorted(r[1] for r in results) == [0, 2, 4, 6, 8, 10, 12, 14] | ||
| # 8 points x 0.05s sequentially would take >=0.4s; with 4 workers | ||
| # it should comfortably finish in well under that. | ||
| assert elapsed < 0.35 | ||
| # More than one worker thread must actually have been used. | ||
| assert len(seen_threads) > 1 |
Comment on lines
+2149
to
+2162
| - calculators must be a positive int (defaults to 1): the number of | ||
| design points evaluated concurrently. calculators=1 (the default) | ||
| calls the function sequentially, one at a time, in the calling | ||
| thread — never via a thread pool. This is required for model | ||
| callables that are only safe to call from that thread (e.g. an R | ||
| function bridged in via reticulate); the fz.R wrapper always forces | ||
| calculators=1 for this reason. calculators=N>1 dispatches calls to | ||
| a thread pool of N worker threads, running evaluations | ||
| concurrently — only use this for an ordinary, thread-safe Python | ||
| function. If any point raises while evaluated in parallel, fzd | ||
| aborts and re-raises it immediately as an explicit RuntimeError | ||
| (rather than silently marking just that point as failed), since a | ||
| thread-safety issue can otherwise surface as sporadic, hard to | ||
| diagnose per-point failures |
test_parallel_runs_across_multiple_threads compared wall-clock elapsed time against a tight 0.35s threshold (8 points x 0.05s / 4 workers ~= 0.1s ideal), which flaked on the slower/more contended macOS CI runner (0.351s observed, see #80 macOS Python 3.12 job). Increase the per-point sleep to 0.1s (0.8s sequential, ~0.2s ideal parallel) and the threshold to 0.6s, giving much more headroom while still clearly proving concurrent execution. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
fzd(..., model=<callable>, calculators=N)previously acceptedNonly for API compatibility: evaluations always ran sequentially, one call at a time, in the calling thread, regardless of its value. This was deliberate (see_run_function_model_design's original docstring): a Python-sideThreadPoolExecutoralways dispatches to a worker thread — even withmax_workers=1— which crashes callables that are only safe to invoke from the thread that created them (e.g. an R closure bridged in viareticulate).This PR makes
calculators=N>1actually parallelize evaluations for an ordinary, thread-safe Python function, while preserving the safe default:calculators=1(the default): unchanged — strictly sequential, one call at a time, in the calling thread, no thread pool at all.calculators=N>1: dispatches evaluations to aconcurrent.futures.ThreadPoolExecutorwithNworker threads, running them concurrently. Only use this for a model function that is safe to call from arbitrary threads.fz.FunctionModelParallelError(aRuntimeErrorsubclass) naming the offending point, thecalculatorsvalue, and suggesting a retry withcalculators=1— instead of silently downgrading it to a per-point failure like sequential mode does for ordinary model errors. A thread-safety problem can otherwise surface as sporadic, hard-to-diagnose per-point failures, so failing the whole run loudly is safer than continuing with partially-corrupted results.calculatorsmust now be a positive int in function-model mode (raisesValueErrorforcalculators <= 0).FunctionModelParallelErroris exported from the top-levelfzpackage.Why
fzdis documented/used withcalculators=Nas if it parallelizes function-model evaluations (see e.g. theBuildingOpt_fz.ipynbnotebook inyannrichet/OptimHome), but until now that parameter was silently ignored in that mode. This PR makes the parameter do what users reasonably expect for plain Python functions, without weakening the existing safety guarantee for non-thread-safe callables (still opt-in viacalculators=1, the default).Companion change
Funz/fz.Rneeds a matching update (see companion PR): now thatcalculators>1has a real effect on this side, the R wrapper must unconditionally forcecalculators=1whenmodelis an R function (R closures bridged in viareticulateare not safe to call from a worker thread), instead of merely warning that the parameter had no effect.Testing
tests/test_fzd_function_model_parallel.py: unit tests for_run_function_model_design(sequential-in-calling-thread, parallel-uses-multiple-threads, parallel-error-aborts-with-explicit-error, result-order preserved) plus end-to-endfzd()tests (default calculators=1, calculators>1 parallelizes, calculators<=0 rejected, non-int rejected, parallel error propagates asFunctionModelParallelError).fzdsuite still green:tests/test_fzd.py,tests/test_fzd_vector_outputs.py,tests/test_fzd_multiobjective.py,tests/test_fzd_calculator_discovery.py(65 passed).