ENH: Add lemke_howson! with caller-supplied workspace - #237
Conversation
Add a mutating variant following the lcp_lemke! pattern of QuantEcon.jl: the output NE and the primary workspace (tableaux, bases) are positional, and the auxiliary workspace (col_bufs, argmins) consists of keyword arguments defaulting to nothing and materialized lazily. With the full workspace supplied, the call performs no allocations for machine-float element types. Same-shaped array pairs and argmins/bases are asserted non-aliasing (argmins is overwritten in each pivoting step before bases is read to determine the leaving variable); col_bufs[1] === col_bufs[2] is documented as allowed. Internally, col_bufs and argmins move out of _lemke_howson_tbl! into parameters, so the capping loop no longer allocates them on every restart, and _initialize_tableaux! no longer allocates its consts vector per restart; _get_mixed_actions is split into an in-place core and an allocating wrapper. lemke_howson becomes a thin allocating wrapper around lemke_howson!, with results unchanged. Measured at n=30 over 2000 random games (medians): 17.9 -> 16.0 us with full workspace; per-call allocation 32.6 KB -> 0. The remaining runtime dispersion across games is algorithmic (iteration counts), not allocation-driven. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
QuantEcon 0.18 delivers the hybrid pivoting kernel of QuantEcon/QuantEcon.jl#397, which lemke_howson inherits through _pivoting!; the small tableaus of bimatrix games are exactly the regime its loop path speeds up (measured: 1.67 -> 1.12 us median at n=10 before the workspace variant is even used). Record the workspace/aliasing/allocation-test conventions in the agent instructions, per the convention of updating them with the change that introduces them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR introduces an allocation-conscious, mutating Lemke–Howson solver entrypoint (lemke_howson!) that lets callers supply preallocated output and workspace arrays, and refactors the internal implementation to thread workspace through the capping and pivoting paths.
Changes:
- Add and export
lemke_howson!, and makelemke_howsona thin allocating wrapper that forwards to it. - Refactor the Lemke–Howson implementation to accept caller-supplied
col_bufs/argminsand to use an in-place mixed-action extraction helper (_get_mixed_actions!). - Add tests for correctness, aliasing rejection, and allocation behavior; bump QuantEcon compat to
0.18; document workspace/allocation conventions.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/lemke_howson.jl |
Adds lemke_howson!, threads workspace through the algorithm, and splits mixed-action extraction into allocating + in-place variants. |
test/test_lemke_howson.jl |
Adds tests for lemke_howson! correctness, full-output parity, capping path, allocation baseline, and aliasing checks. |
src/GameTheory.jl |
Exports the new lemke_howson! API. |
Project.toml |
Updates QuantEcon compat to 0.18. |
.github/copilot-instructions.md |
Records workspace/aliasing/allocation-test conventions for future contributions. |
| @@ -142,17 +203,41 @@ | |||
| throw(ArgumentError("`init_pivot` must satisfy 1 <= k <= $(total_num)")) | |||
| end | |||
There was a problem hiding this comment.
Fixed in b0b7610. The range check is now factored into a _check_init_pivot helper and the message names the keyword: "`init_pivot` must satisfy 1 <= init_pivot <= $total_num". (The k wording predated this PR — the docstring defines k — but refactoring the check was a natural moment to reword it.)
🤖 Generated with Claude Code (Claude Fable 5)
| for pl in 1:2 | ||
| @assert length(NE[pl]) == nums_actions[pl] "NE[$pl] must have length $(nums_actions[pl])" | ||
| @assert size(tableaux[pl]) == (nums_actions[3-pl], total_num+1) "tableaux[$pl] must have size ($(nums_actions[3-pl]), $(total_num+1))" | ||
| @assert length(bases[pl]) == nums_actions[3-pl] "bases[$pl] must have length $(nums_actions[3-pl])" | ||
| end | ||
| @assert !Base.mightalias(NE[1], NE[2]) "NE[1] must not alias NE[2]" | ||
| @assert !Base.mightalias(tableaux[1], tableaux[2]) "tableaux[1] must not alias tableaux[2]" | ||
| @assert !Base.mightalias(bases[1], bases[2]) "bases[1] must not alias bases[2]" | ||
| if col_bufs !== nothing | ||
| for pl in 1:2 | ||
| @assert length(col_bufs[pl]) == nums_actions[3-pl] "col_bufs[$pl] must have length $(nums_actions[3-pl])" | ||
| end | ||
| end | ||
| if argmins !== nothing | ||
| @assert length(argmins) >= max(nums_actions...) "argmins must have length at least $(max(nums_actions...))" | ||
| # argmins is overwritten in each pivoting step before bases[pl] is | ||
| # read to determine the leaving variable | ||
| for pl in 1:2 | ||
| @assert !Base.mightalias(argmins, bases[pl]) "argmins must not alias bases[$pl]" | ||
| end | ||
| end |
There was a problem hiding this comment.
Done in b0b7610. The workspace validation now throws instead of asserting, following both this library's existing convention and Base/stdlib precedent: DimensionMismatch for array-size mismatches (as in the LAPACK wrappers' checks on caller-supplied tau/ipiv) and ArgumentError for aliasing violations (as in circshift!'s "dest and src must be separate arrays"). The exception contract is stated in the lemke_howson! docstring, and the preference for exceptions over @assert in exported-function validation is recorded in .github/copilot-instructions.md.
One note on the stated rationale: there is currently no runtime flag that disables @assert (--assertions=no is a build-time option for C-level asserts), but the Julia manual does reserve the right to skip assertions at various optimization levels, so the recommendation stands for a public API.
🤖 Generated with Claude Code (Claude Fable 5)
| @test_throws AssertionError lemke_howson!((v, v), tabs, bs, gs) | ||
| @test_throws AssertionError lemke_howson!((copy(v), v), | ||
| (tabs[1], tabs[1]), bs, gs) | ||
| @test_throws AssertionError lemke_howson!((copy(v), v), tabs, | ||
| (bs[1], bs[1]), gs) | ||
| @test_throws AssertionError lemke_howson!((copy(v), v), tabs, bs, gs, | ||
| argmins=bs[1]) |
There was a problem hiding this comment.
Updated in b0b7610 accordingly: the aliasing tests now expect ArgumentError, and new tests cover the DimensionMismatch size rejections (one per array argument, including a too-short argmins) plus out-of-range init_pivot for direct lemke_howson! calls.
🤖 Generated with Claude Code (Claude Fable 5)
* Throw DimensionMismatch for size mismatches of caller-supplied arrays and ArgumentError for aliasing violations, following the Base/stdlib convention (LAPACK wrappers for sizes, circshift! for aliasing) * Factor the init_pivot range check into _check_init_pivot and call it in lemke_howson before float(T) and workspace allocation, restoring the validation ordering of the non-mutating version; reword the message to name init_pivot instead of k * Document the exception contract in the lemke_howson! docstring * Add size-rejection and direct init_pivot tests; update aliasing tests to expect ArgumentError * Record the exception preference in copilot-instructions.md Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Fence the workspace example in the lemke_howson! docstring as a julia code block for syntax highlighting, consistent with the other examples in the file * Note that with full_output=Val(true), res.NE aliases the caller-supplied NE * Add a positive test for the documented col_bufs=(buf, buf) case on a square game Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
This PR also received an out-of-band review from ChatGPT (not posted on this thread), which found one error-path regression and made three non-blocking suggestions. Responding to it here for the record: Validation ordering (the blocker): Correct, and fixed in b0b7610. On Fencing the docstring example: Done in 3833048. One correction to the premise: the 4-space-indented block already rendered as a code block in Julia's Markdown (not plain prose), but fencing it as Positive test for
Separately, prompted by Copilot's review, the workspace validation was switched from 🤖 Generated with Claude Code (Claude Fable 5) |
Pure rename, with the content rewrite following in the next commit, so that git rename detection connects the file histories Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Bring benchmark/ in line with the QuantEcon.jl and ContinuousDPs.jl layout: * benchmarks.jl defines SUITE in the standard BenchmarkTools format (usable with PkgBenchmark.jl or AirspeedVelocity.jl) and includes one file per benchmarked module; it is also runnable standalone * Add lemke_howson.jl: end-to-end lemke_howson on random games (n = 10, 100), the capping heuristic (n = 100, 200), and the repeated-solve regime through lemke_howson! with caller-owned arrays, with and without the full auxiliary workspace * Move the existing suites into support_enumeration.jl, repeated_game.jl, and bimatrix_generators.jl (replacing the standalone generators.jl, now included in SUITE), with string keys and a fresh fixed-seed RNG per case * Add GameTheory, Random, and LinearAlgebra to benchmark/Project.toml so the standalone invocation works * Rewrite README.md documenting each case and the standalone and PkgBenchmark workflows; drop the stale checked-in tune.json workflow Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Exclude the generators subgroup from SUITE unless GAMETHEORY_BENCHMARK_GENERATORS=true is set (with PkgBenchmark, through BenchmarkConfig(env=...)): it times game construction rather than equilibrium computation Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Lock in the documented contract that with full_output=Val(true), res.NE is the caller-supplied NE itself, not a copy Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Drop the GAMETHEORY_BENCHMARK_GENERATORS environment variable: define the generators benchmarks unconditionally as their own top-level group GENERATORS_SUITE, kept out of SUITE so that whole-suite runs (standalone, PkgBenchmark, AirspeedVelocity) skip it. To run it, include benchmarks.jl and run(GENERATORS_SUITE) directly, as with any other subset Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
f559160 to
158a21d
Compare
PkgBenchmark only discovers the group named SUITE, so GENERATORS_SUITE is not reachable through benchmarkpkg/judge. Revive benchmark/generators.jl as a dedicated entry point exposing the bimatrix_generators group as its SUITE, selectable with the script keyword Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add the standalone-execution footer of benchmarks.jl to the generators
entry point, so that the generators benchmarks can be run in one shot:
julia --project=benchmark benchmark/generators.jl
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* The generators test suite is from Fearnley, Igwe, and Savani, not von Stengel et al. (Savani and von Stengel is the reference for unit_vector_game only) * sgc_game is the SGC game of Sandholm, Gilpin, and Conitzer, not Savani-von Stengel; state the implied action count (4k-1 = 1999) * Do not claim in the README lead-in that every module file is a subgroup of SUITE (the generators group is separate) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This PR adds a mutating variant
lemke_howson!accepting caller-supplied output and workspace arrays, following thelcp_lemke!pattern of QuantEcon.jl (QuantEcon/QuantEcon.jl#399): the outputNEand the primary workspace (tableaux,bases) are positional, and the auxiliary workspace (col_bufs,argmins) consists of keyword arguments defaulting tonothing, materialized lazily. It is behavior-preserving:lemke_howsonbecomes a thin allocating wrapper with unchanged results, and all existing tests pass unmodified.Changes
lemke_howson!(new, exported): with the full workspace supplied andfull_output=Val(false), the call performs no allocations for machine-float element types (verified by a measured-baseline test, not a hard-coded== 0). Same-shaped array pairs (NE,tableaux,bases) andargmins/basesare validated as non-aliasing and rejected with anArgumentError(array sizes with aDimensionMismatch) —argminsis overwritten in each pivoting step beforebasesis read to determine the leaving variable, so aliasing would corrupt the solution silently;col_bufs[1] === col_bufs[2]is documented as allowed (each pivoting step uses the buffer in isolation).col_bufsandargminsmove out of_lemke_howson_tbl!into parameters, so thecappingheuristic no longer re-allocates them on every restart, and_initialize_tableaux!no longer allocates itsconstsvector per restart;_get_mixed_actionsis split into an in-place core and an allocating wrapper. This also removes the capping-path allocation overhead for plainlemke_howson(at n=30 withcapping=10: 35.3 KB → 32.0 KB per call, now identical to the uncapped path).QuantEcon = "0.18"(released today).lemke_howsoninherits the hybrid pivoting kernel of PERF: Use a plain loop in _pivoting! for small tableaus QuantEcon.jl#397 through_pivoting!— the small tableaus of bimatrix games are exactly the regime its loop path targets. The full test suite passes against QuantEcon v0.18.0.benchmark/is restructured in the QuantEcon.jl style — per-module files under a standardSUITE(lemke_howson,support_enumeration,repeated_game), runnable standalone or through PkgBenchmark — with a newlemke_howsongroup covering the end-to-end solver, the capping heuristic, and the repeated-solve regime oflemke_howson!with and without the full workspace. The bimatrix-generators benchmarks are kept out of the default suite as a separateGENERATORS_SUITE, with a dedicated entry pointbenchmark/generators.jl(standalone or via PkgBenchmark'sscriptkeyword)..github/copilot-instructions.mdas part of this change.Alternative design considered
A
LemkeHowsonWorkspacestruct bundling the seven arrays would obviously simplify the signature (lemke_howson!(ws, g; ...)) and centralize the shape and aliasing checks in one constructor. It is deliberately not adopted here, to keep the change minimal and the API exactly parallel tolcp_lemke!in QuantEcon.jl — the positional-plus-lazy-keywords pattern needs no new public type, and a workspace struct can always be layered on later without breaking this signature, whereas the reverse migration would be churn.Measurements
Medians per call over batches of random
(n, n)games (2000 games for n=10 and 30; 100 games for n=100), same games for all variants:lemke_howsonlemke_howsonlemke_howson!(full workspace)Per-call allocation with full workspace: 0 bytes (from 4.7 KB at n=10, 32.6 KB at n=30, ~0.3 MB at n=100). The kernel effect (middle column vs left) is confined to small games: for n ≳ 90 the tableaus exceed the kernel's dispatch cutoff and take the BLAS path on both versions, so at n=100 the gain is the workspace variant's (−22%).
One honest finding worth recording: the heavy right tail of per-call runtimes over random games (p99 ≈ 15× median at n=30), previously suspected to be GC-driven, is mostly algorithmic — Lemke–Howson iteration counts vary widely across games, and the tail persists for the allocation-free variant. The value of the zero-allocation path is therefore the steady per-call gain and composability: embedding in allocation-free loops, and multithreaded Monte Carlo, where per-call garbage from concurrent solvers compounds GC pressure across threads.
🤖 Generated with Claude Code (Claude Fable 5)