perf(csa): coalesce immutable proposal issuance - #117
Merged
Merged
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
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.
Change
Coalesce queue advancement, proposal-ID allocation, and pending/generation registration into one immutable issuance operation. An ordinary queued proposal now constructs one
CSAEngineStateinstead of three, and oneGenerationRuntimeStateinstead of two.Generation materialization still commits selection, trace, and RNG once per generation. Sampling validation, batch-end provenance binding,
ask()/tell()behavior, and checkpoint formats are unchanged. This does not remove pending-registry copying or change its complexity.Regression coverage exercises custom emission hooks, partial feedback, failed issuance, candidate identity, interleaved state forks, and checkpoint/exact-async continuation. After integrating current
main, all 32 comparison cases still match the baseline's proposals, state, RNG, trace, and bank-update results.Hook Migration
CSAOptimizer.propose_candidate(state)is replaced byemit_proposal(state), returning(proposal, tracks_generation, planned_provenance, state). Overrides must return a fully issuedProposaland a state that already owns its ID and pending registration. Generated children must also advance the queue and register their generation ID. Leave provenance binding toask().No compatibility shim is retained: the old hook exposed a dequeued but unissued state and required separate allocation and registration transitions. The new hook makes issuance one operation. The migration is documented in the changelog and method docstring.
Measurements
The following are pre-integration measurements against
6c65fa5cof the issuance implementation committed in9ce98895. They are not fresh timings of the merged PR head; the 32-case equivalence check was rerun after the merge.Steady ask/tell time per 70 observations, batch size 1, crowding and trace disabled:
The full panel crosses these four shapes with batch sizes 1/8, crowding off/on, and trace off/on: 32 cells, bank capacity 8, seed 11, and two completed generations. Runs were sequential in A/B/B/A order, with two warmups and seven samples per cell per run. The table uses pooled medians of 14 samples per revision; no samples from those four runs were removed.
Across the 32 cells, the median of the per-cell steady-time reductions was 6.9% (range 0.3–15.6%). Initial-bank-inclusive time fell by a median 5.8%. Within-run relative MAD medians were about 2–3%. The unchanged frozen-input bank replay moved by a median 1.4% (range -6.5–5.8%), indicating residual timing noise.
Workloads use
CSAOptimizer.from_space_defaultswith bank growth capped at 8 and only the far-update mode varied for crowding. Real bounds are [-5, 5]; integer bounds are [-8, 8]. The mixed record contains four real leaves, one integer, and a three-choice category. Objectives use squared numeric values, position-weighted sums for arrays, and category indices, without added delay. Timing includes objective calls, observation construction, validation, RNG work, configured trace recording, and result retention; setup, checkpoint serialization, hashing, and instrumentation are outside the timed region.Environment: macOS arm64, Python 3.11.0, NumPy 2.4.6, SciPy 1.17.1, joblib 1.5.3, and cloudpickle 3.1.2; default GC and native thread limits set to 1. The host had substantial competing load, so these are provisional small-bank results, not a universal speedup claim. No peak-memory improvement or comparison with legacy CSA is claimed.