Repository navigation
Conversation
…four element types Owner directive: reduce the tests, not what is tested, and test all four element types within a 10-minute CI budget. What changed (every cut carries a rationale comment at the site): * Int64 index-type legs run on one real + one complex element type through int_types(T) (test/utils.jl): an Int64 kernel instance differs in the index type, not the precision, so one part per element-type class covers it while the others skip a second compilation of every kernel family. Host-side index-semantics tests keep the full INTTYPES loop. The ported suite runs Int32 only, as CUDSS.jl itself tests, with each feature's Int64 coverage in its owning file. * Full option crosses replaced by covering subsets: LDL/LU non-default regime mixes re-check a covering case subset; ported view x pivot crosses keep one cell per distinct branch; refinement's structure x mode x steps cross keeps every mode with and without refinement on both structures; solve/batch nrhs and batch-size values that exercise identical kernels deduplicated. * Repeated seeded random trials reduced where the test is not about variability (MC64 brute-force: 12 -> 4 draws); two LDLt case matrices dropped as redundant with retained cases (ownership documented); two compile-only heavyweight factorize! calls replaced by regime-C-only analyses that check the same acceptance property. * CI: pull-request and push runs now test ALL FOUR element types (the Float64,ComplexF32 restriction is gone; owner call). Validation on shin-compute-001 (32 workers, all four element types, CPU): baseline (main) 71977/71977 SUCCESS in 11m32.3s diet 63565/63565 SUCCESS in 10m53.6s Test CPU-seconds 18530 -> 18302 on that box; the per-file deltas are noisy (shared box), so the authoritative wall-time verdict is this PR's own CI cycle on the kkt runner, which this PR intentionally runs at the full four-type selection. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* The CI selection contract text now matches the all-four-eltypes reality in
all three places that stated the old two-type PR selection (ci.yml comment,
AGENTS.md twice, backends.jl comment).
* INT64_ELTYPES prefers the 8-byte carriers (Float64/ComplexF64) when
selected, so the historically tested 8-byte-T x Int64 handle-layer pairs
survive the default selection; the docstring states honestly what the
API-layer Int64 legs lose and where the deep numeric Int64 coverage lives;
explicit error on a real-free selection instead of a bare first(()).
* The MC64 brute-force trials go back to 12: the testset is host-only and
compile-dominated, so the cut saved nothing while removing fuzz coverage.
* Coverage-ownership comments extended where the review found the claims
under-specified: the given-up elsize side effect in cholesky_a, the
symmetric-Schur Int64 story, the vacuous ('U','N') ported cell, the LU 'R'
drop, the ldlt solve-subset axes, and FGMRES's INT-agnostic extension.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Author
|
Adversarial review (full findings applied in the follow-up commit): the stale two-eltype contract text is updated in ci.yml/AGENTS.md/backends.jl; |
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.
Owner directive: reduce the tests, not what is tested, with a 10-minute CI budget at all four element types.
What changed
int_types(T)(test/utils.jl): Int64 kernel instances differ in the index type, not the precision, so the Int64 legs run on one real + one complex element type; host-side index-semantics tests keep the full loop; the ported suite runs Int32 only (as CUDSS.jl itself tests) with each feature's Int64 coverage in a named owning file.factorize!calls replaced by regime-C-only analyses checking the same acceptance property.Float64,ComplexF32PR restriction is removed — this PR's own CI cycle therefore measures the target configuration directly.Not touched: no element type, backend, or tolerance removed from any loop; no sole-coverage testset deleted; the pivot sequences and reference comparisons are unchanged.
Measured (shin-compute-001, 32 workers, CPU backend, all four element types)
That box is critical-path-bound at 32 workers, so the per-file deltas are noisy there; the authoritative wall-time verdict against the 10-minute budget is this PR's CI on the
kktrunner, running at the full four-type selection. If it lands over budget, the next lever is splitting the dominant files into more SPLIT_FILES parts for better worker packing — follow-up material.🤖 Generated with Claude Code