fix(#30): phase-aware concurrency control for Sobol vs BO phases#32
Open
blttkgl wants to merge 1 commit into
Open
fix(#30): phase-aware concurrency control for Sobol vs BO phases#32blttkgl wants to merge 1 commit into
blttkgl wants to merge 1 commit into
Conversation
Resolves #30 Previously, backend.max_parallelism was never linked to manager.job_limit, so Ax silently capped concurrent trial generation at DEFAULT_BO_CONCURRENCY=3 regardless of the scheduler's actual capacity. Changes: - Session._apply_backend_preferences() wires manager.job_limit to backend.max_parallelism so the Sobol initialization phase can fill all available scheduler slots - Add Session.bo_concurrency parameter (Optional[int]) to cap concurrent acquisitions during the BO phase independently of job_limit - Phase detection in _is_bo_phase() checks Ax's actual experiment trial count (not just finished cases) to match Ax's internal Sobol→BO transition - _validate_bo_concurrency() enforces type correctness at construction time - Persist/restore bo_concurrency through session config - 6 new tests covering Sobol-phase pass-through, BO-phase capping, max_parallelism sync on start() and restore() - Update aerofoilNACA0012Steady example with bo_concurrency=2, job_limit=10 Signed-off-by: blttkgl <buluttekgul@gmail.com>
Owner
|
Hey @blttkgl, I reviewed the PR yesterday and had some findings; I can address those tonight on this branch if you'd want to. Main nits have to do with the |
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.
Title:
fix(#30): phase-aware concurrency control for Sobol vs BO phasesDescription:
Fixes #30.
max_parallelismwas never linked tomanager.job_limit, so Ax silently capped concurrent trial generation at 3 regardless of the scheduler's actual capacity.What changed
Session._apply_backend_preferences()now wiresmanager.job_limit→backend.max_parallelism, allowing the Sobol initialization phase to fill all available scheduler slots.Session(bo_concurrency=N)parameter caps concurrent acquisition during the BO phase independently ofjob_limit. Defaults toNone(uncapped)._is_bo_phase()checks Ax's actual experiment trial count to match Ax's internal Sobol→BO transition (counting pending trials, not just finished ones).bo_concurrencypersists/restores through session config.bo_concurrency=2, job_limit=10.Behaviour
job_limit(full scheduler capacity)bo_concurrencyif set, elsejob_limitFully backward-compatible — existing sessions without
bo_concurrencybehave as before, except Sobol now uses all available slots.