PERF: Use a plain loop in _pivoting! for small tableaus - #397
Conversation
A rank-1 update has no data reuse for BLAS kernels to exploit, so a plain SIMD loop matches single-threaded BLAS throughput, while below the crossover where BLAS multithreading amortizes its synchronization latency (~100 rows; machine dependent only through a square root of the hardware constants) the loop also avoids the call overhead. Dispatch on PIVOTING_BLAS_CUTOFF = 64 rows: 28% faster at the cutoff, 2-4x faster at n <= 16, unchanged above. Non-BLAS eltypes now take the loop path instead of the broadcast fallback. Measured on lcp_lemke with random positive definite M: 40% faster at n=10, 9% at n=50, unchanged at n >= 96. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Cases around the loop/BLAS dispatch of _pivoting!: n = 10 and 50 on the loop kernel, n = 200 on the BLAS kernel, plus a preallocated lcp_lemke! case for the repeated-solve regime. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR improves performance of the pivoting step used by Lemke-style algorithms by introducing a fast plain-loop kernel for small tableaus while preserving the existing BLAS-based update for larger ones.
Changes:
- Added
PIVOTING_BLAS_CUTOFF = 64and dispatch in_pivoting!between a new SIMD loop kernel and the existing BLAS (scal!+ger!) kernel. - Added tests ensuring the two kernels agree up to roundoff,
_pivoting!selects the expected kernel across the cutoff, and the pivot column is exactly reduced to a unit vector. - Added a new
benchmark/lcp_lemke.jlsuite and hooked it intobenchmark/benchmarks.jl, with README documentation for the new suite.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/pivoting.jl |
Adds loop-vs-BLAS kernel dispatch for _pivoting! and implements the new loop kernel. |
test/test_pivoting.jl |
Adds coverage comparing loop and BLAS kernels and verifying kernel selection and exact pivot-column cleanup. |
benchmark/lcp_lemke.jl |
Introduces an end-to-end benchmark suite for lcp_lemke / lcp_lemke! around the pivoting cutoff. |
benchmark/benchmarks.jl |
Registers the new lcp_lemke benchmark group in the top-level suite. |
benchmark/README.md |
Documents the new lcp_lemke benchmark group and its cases. |
The loop/BLAS crossover tracks the memory traffic of the rank-1 update, i.e. the total tableau size, not the row count: measured on rectangular shapes, BLAS is 2-3x faster at (64, 1000) and (64, 10000), which the row-count criterion routed to the loop. Rectangular tableaus arise downstream in GameTheory.jl, where Lemke-Howson tableaus have shape (n, m+n+1). Dispatch on length(tableau) > 2^14, which classifies every measured square and rectangular point correctly. Also assert that the lcp_lemke benchmark fixtures perform at least one pivot, and list the suite in the README overview. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Multiplying by inv(pivot_elt) overflows for eltypes whose reciprocal range the pivot tolerances do not protect: for Float16, inv overflows for pivots below 1/floatmax(Float16) ~ 1.5e-5, turning zero pivot-row entries into NaN via 0 * Inf, where the rdiv!-based fallback this kernel replaced stayed finite. Multiply by the reciprocal only for BlasFloat eltypes (matching the BLAS kernel), and divide directly otherwise; the type-level branch specializes away. Found in external review of #397. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Applied the findings of an external review by ChatGPT, in the two commits above: FIX: direct division for non-BLAS eltypes (97f1f59). The reviewer found that the loop kernel's reciprocal normalization overflows for eltypes whose reciprocal range the pivot tolerances do not protect, and supplied a PERF: dispatch on total tableau size (8411475). The reviewer questioned the row-count dispatch criterion for rectangular tableaus, such as the Minor points, all applied: the dispatch boundary is tested exactly at Validation: 🤖 Generated with Claude Code (Claude Fable 5) |
- Restore dense_n10_prealloc to its #397 workload (caller-owned output arrays, default keywords), keeping the benchmark key comparable across commits, and add dense_n10_full_workspace for the allocation floor. - Scope the allocation claims: with full workspace the call performs no workspace allocations; being fully allocation-free additionally requires machine-float eltypes (BigFloat arithmetic allocates), and result-struct elision is context-dependent. - Assert that argmins does not alias basis: _lex_min_ratio_test! overwrites argmins before basis[pivrow] is read to determine the leaving variable, so aliasing silently corrupts the solution. Add a regression test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…399) * PERF: Make lcp_lemke! allocation-free with caller-supplied workspace The preallocating variant still allocated internally: the col_buf and argmins workspace vectors, and BitVector temporaries from the broadcasted input checks. Expose col_buf and argmins as keyword arguments with allocating defaults (backward compatible), and replace the broadcasted checks with predicate forms, preserving their NaN behavior. With d, col_buf, and argmins supplied, lcp_lemke! performs no allocations, so repeated solves generate no garbage-collector pressure: 12 allocations -> 0 at n=10, with the default path dropping to 6 from the check fixes alone. The dense_n10_prealloc benchmark case now supplies the full workspace and measures the allocation-free repeated-solve floor. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * TEST: Bound the lcp_lemke! allocation test by a measured baseline Julia 1.10 (LTS) heap-allocates the returned LCPResult struct (a non-isbits immutable), where 1.12 and later stack-allocate it, so the flat == 0 assertion failed on the LTS CI jobs. Instead of hard-coding the struct's boxed size (which encodes layout, GC pool geometry, and platform word size), assert that the call allocates at most as much as one LCPResult construction measured in the same escape pattern; on newer versions that baseline is itself zero. Qualify the docstring claim accordingly. The workspace itself allocates nothing on any version. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * PERF: Materialize lcp_lemke! keyword defaults lazily Keyword defaults evaluate at function entry whenever the keyword is not supplied, so the allocating defaults for col_buf and argmins (added on this branch) and for d (pre-existing) were paid even by calls taking the trivial all(q .>= 0) early return. Default the three keywords to nothing and materialize them only after the trivial-case check; supplied arguments are still validated up front. The trivial path with default keywords now allocates nothing beyond the returned LCPResult, better than master, which allocated d and two BitVector temporaries there. Found by Copilot review of #399. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * MAINT: Apply ChatGPT review comments on #399 - Restore dense_n10_prealloc to its #397 workload (caller-owned output arrays, default keywords), keeping the benchmark key comparable across commits, and add dense_n10_full_workspace for the allocation floor. - Scope the allocation claims: with full workspace the call performs no workspace allocations; being fully allocation-free additionally requires machine-float eltypes (BigFloat arithmetic allocates), and result-struct elision is context-dependent. - Assert that argmins does not alias basis: _lex_min_ratio_test! overwrites argmins before basis[pivrow] is read to determine the leaving variable, so aliasing silently corrupts the solution. Add a regression test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…400) Three additions learned from review and validation during the recent pivoting/lcp_lemke performance work: - Benchmarks: never change the workload behind an existing benchmark key (judge compares keys across commits); add a new key instead. - Performance work: check aliasing hazards among caller-supplied workspace arrays and assert non-aliasing where corruption would be silent (argmins/basis in lcp_lemke!). - Validation: run each scenario in its own process; scenario 3 binds `u`, which collides with scenario 4's example file in a shared process. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* ENH: Add lemke_howson! with caller-supplied workspace 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> * MAINT: Require QuantEcon 0.18; record workspace conventions 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> * MAINT: Validate lemke_howson! inputs with exceptions, not @Assert * 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> * DOC/TEST: Polish lemke_howson! docs; test shared col_buf case * 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> * MAINT: Rename benchmark/generators.jl to bimatrix_generators.jl 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> * MAINT: Restructure benchmark suite; add lemke_howson benchmarks 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> * MAINT: Make the bimatrix_generators benchmarks opt-in 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> * TEST: Assert res.NE identity with caller-owned NE 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> * MAINT: Replace generators env-var opt-in with separate GENERATORS_SUITE 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> * MAINT: Add PkgBenchmark entry point for the generators benchmarks 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> * MAINT: Make generators.jl runnable standalone 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> * DOC: Fix generator attributions in benchmark docs * 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> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This PR speeds up the pivoting step shared by
lcp_lemkeand (via GameTheory.jl)lemke_howson, following the performance review ofpivoting.jl/lcp_lemke.jl(#368/#371).Changes
_pivoting!dispatches between two kernels on total tableau size (PIVOTING_BLAS_CUTOFF = 2^14elements; revised from a row-count criterion after review — the crossover tracks the memory traffic of the update, which row count misrepresents for the rectangular tableaus that arise in GameTheory.jl): at most 2^14 elements, a single fused SIMD loop in column-major order (dividing directly for non-BLAS eltypes such asFloat16, whose reciprocals can overflow); above, the existingBLAS.scal!+BLAS.ger!path, unchanged. The function signature is unchanged, solcp_lemkeand GameTheory.jl'slemke_howson(which imports_pivoting!and passescol_buf) need no changes and inherit the speedup. Non-BLAS eltypes (e.g.BigFloat) now take the loop path instead of the broadcast fallback.benchmark/lcp_lemke.jlsuite is added, with end-to-end cases on both sides of the kernel dispatch and a preallocatedlcp_lemke!case for the repeated-solve regime._pivoting!selects the intended kernel on both sides of the cutoff, and the pivot column is reduced to the exact unit vector.Rationale for the cutoff
A rank-1 update has no data reuse for BLAS kernels to exploit, so a plain SIMD loop matches single-threaded BLAS throughput exactly (measured: 268 μs for both at n=1000); the only structural advantage of BLAS is multithreading, which pays only where the work amortizes the thread-pool synchronization latency. The crossover size depends on the machine only through a square root of the hardware constants (sync latency, cache-level bandwidth, bandwidth scaling across cores), so a fixed constant is defensible. The crossover tracks the total tableau size (the memory traffic of the update) rather than the row count — confirmed by rectangular measurements: BLAS is 2.2x faster at (64, 1000) and 3.0x at (64, 10000), shapes that a row-count criterion would route to the loop. 2^14 elements errs low, which is the cheap direction: using the loop slightly below the true crossover costs a few percent, while the BLAS side is byte-identical to the current implementation.
Kernel crossover measured on three platforms (loop/BLAS time ratio; < 1 means the loop is faster):
macOS and Linux agree that the crossover lies between 64 and 96 rows on these square
(n, 2n+2)tableaus, i.e. between ~8k and ~19k elements, bracketing the 2^14 cutoff. On the Windows CI runner, OpenBLAS's threadedger!has pathological synchronization overhead (15 μs at n=64 against 1 μs for the loop) and the loop wins through at least n=256; below the cutoff this PR shields that platform from the pathology, above it the behavior is the status quo. An OS-conditional cutoff is left as a possible follow-up pending measurements on non-virtualized Windows hardware.Benchmarks
lcp_lemkeon random positive definite LCPs (minimum times, this machine): 40% faster at n=10 (0.9 → 0.54 μs), 9% at n=50, unchanged at n ≥ 96 (BLAS path). New suite baseline:dense_n100.40 μs,dense_n10_prealloc0.35 μs (12 allocations — removing these is planned as a follow-up PR onlcp_lemke!workspace),dense_n5016.9 μs,dense_n200456 μs.🤖 Generated with Claude Code (Claude Fable 5)