diff --git a/CHANGELOG.md b/CHANGELOG.md index de9c4d4b9..f9fc3a167 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,8 @@ Dependency realignment onto the released control-toolbox ecosystem and a full do - **Export-collision canary**: a regression test pins `intersect(names(ExaModels), names(OptimalControl))` to its known genuine-clash set `[:constraint, :objective]`, so the next colliding upstream export lands as a red test rather than a user bug report +- **GPU capability checks aligned with the CTSolvers contract** ([#883](https://github.com/control-toolbox/OptimalControl.jl/issues/883)). `TestCapabilities` moved into `test/runtests.jl` as `Main`-bound constants (`CUDA_FUNCTIONAL`, `ON_GPU_RUNNER`, `GPU_EXTENSION_ARMED`), replacing the `test/helpers/capabilities.jl` functions; `ON_GPU_RUNNER` now matches the `kkt` / `occidata` *substring* of `RUNNER_NAME` (the self-hosted runners register as `…-runner`, [CTSolvers#223](https://github.com/control-toolbox/CTSolvers.jl/pull/223)). A new `test/suite/environment/test_environment_contract.jl` centralises the loud device requirement on the GPU runner and greps the suite for the `isdefined`-against-`Main` and unbraced-device-guard anti-patterns. Fixes the asymmetry where a lost GPU device failed loudly in `test_gpu_routing.jl` but skipped silently in `test_options_forwarding.jl` + ### Documentation - The documentation site was rewritten onto a new sitemap (getting started, modelling, solve, results, flows, geometry, an examples gallery, and a thematic API reference) and a v2.0 → v2.1 migration page added. The retired v2.0 manuals are kept under `docs/attic/` diff --git a/test/helpers/capabilities.jl b/test/helpers/capabilities.jl deleted file mode 100644 index 883f12bf3..000000000 --- a/test/helpers/capabilities.jl +++ /dev/null @@ -1,65 +0,0 @@ -# ============================================================================ -# Test capabilities -# ============================================================================ -# Mirrors the `TestCapabilities` pattern introduced upstream in CTSolvers. -# -# Two questions that are easy to conflate and must not be: -# -# `gpu_extension_armed()` — is the `CTSolversMadNLPGPU` extension loaded? -# CPU-runnable. It is the only local evidence that -# the GPU code path is even compiled in. -# `is_cuda_on()` — is there a functional GPU *device*? -# False on every CI runner we have locally. -# -# ⚠️ Since CTSolvers#189 the extension trigger is -# `CTSolversMadNLPGPU = ["MadNLPGPU", "CUDA", "CUDSS"]` — all three. Loading -# `MadNLPGPU` and `CUDA` without `CUDSS` leaves the extension inactive and -# `MadNLPGPU{GPU}` unregistered as a strategy, silently. -# -# `is_cuda_on()` used to be defined three times independently -# (runtests.jl — unused, test_canonical.jl, test_options_forwarding.jl). -# This file is the single definition. -# -# Included (not `using`-ed) by the test files that need it, since TestRunner -# runs files independently and each must stand alone. - -module TestCapabilities - -using CUDA: CUDA -using CTSolvers: CTSolvers - -""" - is_cuda_on() - -`true` when a functional CUDA device is present. Guards *device* runs only. -Prefer `Test.@test_skip` over a silent `if is_cuda_on()` so skipped assertions -appear in the summary instead of vanishing. -""" -is_cuda_on() = CUDA.functional() - -""" - gpu_extension_armed() - -`true` when `CTSolversMadNLPGPU` is loaded — i.e. `MadNLPGPU`, `CUDA` *and* -`CUDSS` are all in the session. CPU-runnable: it says nothing about whether a -device exists. -""" -gpu_extension_armed() = Base.get_extension(CTSolvers, :CTSolversMadNLPGPU) !== nothing - -""" - on_gpu_runner() - -`true` on the self-hosted `kkt` or `occidata` GPU runner. - -`RUNNER_NAME` is set by the GitHub Actions runner agent itself, so this needs no -CI.yml or CTActions change. Its purpose is to turn the device tier from -*skipped* into *required* on the machines that must have a device: without -it, a runner whose driver broke is indistinguishable from a laptop, and the GPU -job goes green having run nothing — the failure class of CTSolvers#189. - -If a runner is ever renamed this check stops firing silently rather than -failing loudly; update the names alongside CI.yml. -""" -on_gpu_runner() = get(ENV, "RUNNER_NAME", "") in ("kkt", "occidata") - -end # module diff --git a/test/runtests.jl b/test/runtests.jl index 0173f4fcf..aab11b9f1 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -21,13 +21,47 @@ const SHOWTIMING = true end using .TestOptions: VERBOSE, SHOWTIMING -# CUDA availability check -using CUDA -is_cuda_on() = CUDA.functional() -if is_cuda_on() - @info "✅ CUDA functional, GPU tests enabled" +# Capability constants computed once, here, where a top-level `using` is guaranteed to bind +# into Main. Test files read `Main.TestCapabilities.*` instead of `isdefined(Main, :X)` +# checks or local predicates: `using X` inside a suite/*/test_*.jl file's own module binds X +# into that submodule, not Main, so such checks are always false regardless of what is loaded. +# Mirrors the `TestCapabilities` pattern in CTSolvers (control-toolbox/CTSolvers.jl#190/#223). +# +# CUDA_FUNCTIONAL — is there a functional GPU *device*? The suite's single device +# predicate — never define a local `is_cuda_on()` in a test file. +# Guard device runs with `Test.@test_skip` on the `else`, never a +# silent `if CUDA_FUNCTIONAL` with no branch (see Handbook +# philosophy/testing.md §"Capability-gated tests"). +# ON_GPU_RUNNER — turns the device tier from *skipped* into *required* on the +# self-hosted GPU runners. `RUNNER_NAME` is set by the GitHub Actions +# runner agent itself (no CI.yml change needed) to the runner's +# *registered* name — `kkt-runner` / `occidata-runner` for ours (the +# CI.yml `runs_on` label is the bare `kkt`/`occidata`), so match the +# `kkt`/`occidata` substring to survive the `-runner` suffix. +# GPU_EXTENSION_ARMED — is `CTSolversMadNLPGPU` loaded? CPU-runnable: it says nothing about +# a device, only that the GPU code path is compiled in. Since +# CTSolvers#189 the trigger is `MadNLPGPU` + `CUDA` + `CUDSS` — all +# three; loading two of them leaves it inactive, silently. +# +# Enforcement lives centrally in test/suite/environment/test_environment_contract.jl. +module TestCapabilities +using CUDA: CUDA +using CUDSS: CUDSS # with CUDA + MadNLPGPU, arms CTSolversMadNLPGPU +using MadNLPGPU: MadNLPGPU +using CTSolvers: CTSolvers + +const CUDA_FUNCTIONAL = CUDA.functional() +const ON_GPU_RUNNER = any( + gpu -> occursin(gpu, get(ENV, "RUNNER_NAME", "")), ("kkt", "occidata") +) +const GPU_EXTENSION_ARMED = + Base.get_extension(CTSolvers, :CTSolversMadNLPGPU) !== nothing +end + +if TestCapabilities.CUDA_FUNCTIONAL + @info "✅ CUDA functional, GPU device tests enabled" else - @info "⚠️ CUDA not functional, GPU tests will be skipped" + @info "⚠️ CUDA not functional, GPU device tests will skip (Test.@test_skip)" end # Run tests using the TestRunner extension diff --git a/test/suite/builders/test_options_forwarding.jl b/test/suite/builders/test_options_forwarding.jl index b69ecba1c..3ed7d5672 100644 --- a/test/suite/builders/test_options_forwarding.jl +++ b/test/suite/builders/test_options_forwarding.jl @@ -18,10 +18,6 @@ using CUDA: CUDA include(joinpath(@__DIR__, "..", "..", "problems", "TestProblems.jl")) import .TestProblems -# Shared CUDA/GPU capability checks — one definition for the whole suite. -include(joinpath(@__DIR__, "..", "..", "helpers", "capabilities.jl")) -using .TestCapabilities: is_cuda_on - const VERBOSE = isdefined(Main, :TestOptions) ? Main.TestOptions.VERBOSE : true const SHOWTIMING = isdefined(Main, :TestOptions) ? Main.TestOptions.SHOWTIMING : true @@ -52,8 +48,10 @@ function test_options_forwarding() end # --- backend: CUDA backend, device tier --- - # `Test.@test_skip` rather than a silent `if is_cuda_on()`: a skip - # shows up in the summary, a missing branch does not. + # `Test.@test_skip` on the `else` rather than a silent `if`: a skip + # shows up in the summary, a missing branch does not. A lost device + # on a GPU runner is caught centrally by + # test/suite/environment/test_environment_contract.jl. Test.@testset "backend (CUDA)" begin gpu_x0_is_cuarray() = begin modeler = OptimalControl.Exa{OptimalControl.GPU}(backend=CUDA.CUDABackend()) @@ -61,7 +59,7 @@ function test_options_forwarding() # With CUDA backend, x0 should be a CUDA array nlp.meta.x0 isa CUDA.CuArray end - if is_cuda_on() + if Main.TestCapabilities.CUDA_FUNCTIONAL Test.@test gpu_x0_is_cuarray() else Test.@test_skip gpu_x0_is_cuarray() diff --git a/test/suite/environment/test_environment_contract.jl b/test/suite/environment/test_environment_contract.jl new file mode 100644 index 000000000..38a7577ce --- /dev/null +++ b/test/suite/environment/test_environment_contract.jl @@ -0,0 +1,145 @@ +# ============================================================================ +# Test-environment contract +# ============================================================================ +# Central enforcement of the Handbook's capability-gated-test policy +# (philosophy/testing.md §"Capability-gated tests"). Mirrors the equivalent +# file in CTSolvers (control-toolbox/CTSolvers.jl#190/#223). +# +# Three things are pinned here so they cannot regress silently: +# +# 1. The GPU solver extension is armed on *every* runner (packages loaded). +# 2. On a self-hosted GPU runner, a missing/broken device fails loudly here +# rather than being skipped everywhere else. +# 3. Two anti-patterns stay out of the suite: an `isdefined` check against +# `Main` for a package symbol (always false — `Base.include(Main, file)` +# only binds each test file's own module into `Main`, not what it `using`s +# internally), and an unbraced device-predicate guard around the only +# assertions in a testset (a green testset with zero assertions is +# indistinguishable from a real pass). This file is excluded from both +# walks — it necessarily names the patterns it searches for. + +module TestEnvironmentContract + +using Test: Test + +const VERBOSE = isdefined(Main, :TestOptions) ? Main.TestOptions.VERBOSE : true +const SHOWTIMING = isdefined(Main, :TestOptions) ? Main.TestOptions.SHOWTIMING : true + +""" + _isdefined_main_offenders() + +Recursively find, under `test/suite/` (located via `@__DIR__`, not `pwd()`), source lines +matching `isdefined(Main, ...)` for a package symbol other than the two legitimate idioms: + +- `TestOptions` — the `VERBOSE`/`SHOWTIMING` idiom at the top of every suite test file; + `TestOptions` is a module defined directly in `test/runtests.jl`, i.e. genuinely in `Main`. +- `CUDA` — `test/runtests.jl`'s `TestCapabilities` module does `using CUDA`, and a bare + `using` at that level also leaks the name into `Main`; unlike every other package, which + is only ever `using`'d *inside* a suite file's own module and so never reaches `Main`. + +Any other such check is the anti-pattern this file exists to catch +(control-toolbox/CTSolvers.jl#189): always `false` no matter what is loaded. This file is +excluded from the walk — it names the pattern it searches for. +""" +function _isdefined_main_offenders() + suite_dir = joinpath(@__DIR__, "..") + offenders = Tuple{String,Int,String}[] + pattern = r"isdefined\(Main,\s*:(\w+)\)" + this_file = basename(@__FILE__) + for (root, _, files) in walkdir(suite_dir) + for f in files + (endswith(f, ".jl") && f != this_file) || continue + path = joinpath(root, f) + for (lineno, line) in enumerate(eachline(path)) + m = match(pattern, line) + if m !== nothing && m.captures[1] ∉ ("TestOptions", "CUDA") + push!(offenders, (relpath(path, suite_dir), lineno, m.captures[1])) + end + end + end + end + return offenders +end + +""" + _silent_cuda_guard_offenders() + +Recursively find, under `test/suite/` (located via `@__DIR__`, not `pwd()`), lines that open +an `if` block directly on a CUDA-device predicate — a local `is_cuda_on()` call or a bare +`CUDA.functional()` call — the anti-pattern that makes a correctly-skipped run (no device, +as expected on a developer machine) and a silently-broken run (device *should* be present +but isn't) produce the same output: a green testset with zero assertions (Handbook +`philosophy/testing.md` §"Capability-gated tests", control-toolbox/CTSolvers.jl#189). + +The sanctioned form is `if Main.TestCapabilities.CUDA_FUNCTIONAL ... else Test.@test_skip +... end`, with the device tier made *required* on the GPU runners centrally, in the testset +below. This file is excluded from the walk: it necessarily spells out the very patterns it +searches for. +""" +function _silent_cuda_guard_offenders() + suite_dir = joinpath(@__DIR__, "..") + offenders = Tuple{String,Int,String}[] + # Assembled from two literals so this line does not match itself. + pattern = Regex("if\\s+(is_cuda_on\\(\\)|CUDA" * "\\.functional\\(\\))") + this_file = basename(@__FILE__) + for (root, _, files) in walkdir(suite_dir) + for f in files + (endswith(f, ".jl") && f != this_file) || continue + path = joinpath(root, f) + for (lineno, line) in enumerate(eachline(path)) + if match(pattern, line) !== nothing + push!(offenders, (relpath(path, suite_dir), lineno, strip(line))) + end + end + end + end + return offenders +end + +function test_environment_contract() + Test.@testset "Test-environment contract" verbose = VERBOSE showtiming = SHOWTIMING begin + Test.@testset "GPU solver extension is armed" begin + # Runs on every runner, including CPU-only laptops: "armed" comes from packages + # being loaded (Project.toml + `using` in test/runtests.jl), not from a driver. + # This is the assertion that catches CTSolvers#189 itself — it fails the moment + # CUDSS falls out of the extension trigger wiring again. + Test.@test Main.TestCapabilities.GPU_EXTENSION_ARMED + end + + Test.@testset "GPU driver required on the GPU runner" begin + # On a machine that is supposed to have a GPU, a missing/broken device fails + # loudly here rather than being silently skipped everywhere else. + # + # `RUNNER_NAME` is set automatically by the GitHub Actions runner agent to the + # runner's registered name — `kkt-runner` / `occidata-runner` for our self-hosted + # GPU runners (the CI.yml `runs_on` label is the bare `kkt`/`occidata`). + # `ON_GPU_RUNNER` (test/runtests.jl) matches the `kkt`/`occidata` substring, so it + # survives the `-runner` suffix; past a further rename the check stops firing + # silently rather than failing loudly — keep it in sync with .github/workflows/CI.yml. + if Main.TestCapabilities.ON_GPU_RUNNER + Test.@test Main.TestCapabilities.CUDA_FUNCTIONAL + end + end + + Test.@testset "isdefined(Main, ...) anti-pattern has not returned" begin + offenders = _isdefined_main_offenders() + Test.@test isempty(offenders) + for (file, lineno, sym) in offenders + @warn "isdefined(Main, :$sym) anti-pattern at $file:$lineno — use Main.TestCapabilities instead" + end + end + + Test.@testset "silent CUDA-guard anti-pattern has not returned" begin + offenders = _silent_cuda_guard_offenders() + Test.@test isempty(offenders) + for (file, lineno, text) in offenders + @warn "silent CUDA guard at $file:$lineno — use Main.TestCapabilities.CUDA_FUNCTIONAL with a Test.@test_skip else branch" text + end + end + end +end + +end # module + +# Redefine in outer scope for TestRunner +test_environment_contract() = TestEnvironmentContract.test_environment_contract() diff --git a/test/suite/flows/test_gpu_routing.jl b/test/suite/flows/test_gpu_routing.jl index 93c29036f..f31c4a152 100644 --- a/test/suite/flows/test_gpu_routing.jl +++ b/test/suite/flows/test_gpu_routing.jl @@ -40,9 +40,6 @@ using MadNLP: MadNLP using ExaModels: ExaModels import OrdinaryDiffEqTsit5: OrdinaryDiffEqTsit5 # `Flow` needs an integrator -include(joinpath(@__DIR__, "..", "..", "helpers", "capabilities.jl")) -using .TestCapabilities: is_cuda_on, gpu_extension_armed, on_gpu_runner - const VERBOSE = isdefined(Main, :TestOptions) ? Main.TestOptions.VERBOSE : true const SHOWTIMING = isdefined(Main, :TestOptions) ? Main.TestOptions.SHOWTIMING : true @@ -56,7 +53,7 @@ function test_gpu_routing() # ==================================================================== Test.@testset "GPU extension armed" begin - Test.@test gpu_extension_armed() + Test.@test Main.TestCapabilities.GPU_EXTENSION_ARMED # The single most load-bearing assertion in this file: it is the # only local evidence the GPU code path exists at all, and it # passes with `CUDA.functional() == false`. @@ -163,8 +160,8 @@ function test_gpu_routing() # ==================================================================== Test.@testset "device runs" begin - # `@test_skip` rather than a silent `if is_cuda_on()`: a skipped - # assertion is visible in the summary, an elided branch is not. + # `@test_skip` on the `else` rather than a silent unbraced guard: a + # skipped assertion is visible in the summary, an elided branch is not. gpu_solve() = begin ocp = @def begin t ∈ [0, 1], time @@ -189,17 +186,12 @@ function test_gpu_routing() successful(sol) end - # ⚠️ A skip is honest on a laptop and a lie on a GPU runner. The - # `kkt` and `occidata` runners exist to have a device; if CUDA is - # not functional there, skipping turns the whole GPU job green with - # nothing run — the failure class CTSolvers#189/#190 was about, one - # level up. So on those runners the device itself is asserted, not - # assumed. - if on_gpu_runner() - Test.@test is_cuda_on() - end - - if is_cuda_on() + # A skip is honest on a laptop and a lie on a GPU runner. That the + # `kkt` / `occidata` runners actually have a working device is + # asserted centrally, once, in + # test/suite/environment/test_environment_contract.jl — so here the + # device tier is a plain skip when absent. + if Main.TestCapabilities.CUDA_FUNCTIONAL Test.@test gpu_solve() else Test.@test_skip gpu_solve() diff --git a/test/suite/solve/test_canonical.jl b/test/suite/solve/test_canonical.jl index 64fe47371..de84c83d6 100644 --- a/test/suite/solve/test_canonical.jl +++ b/test/suite/solve/test_canonical.jl @@ -34,10 +34,6 @@ using CUDSS: CUDSS include(joinpath(@__DIR__, "..", "..", "problems", "TestProblems.jl")) using .TestProblems -# Shared CUDA/GPU capability checks — one definition for the whole suite. -include(joinpath(@__DIR__, "..", "..", "helpers", "capabilities.jl")) -using .TestCapabilities: is_cuda_on, gpu_extension_armed - const VERBOSE = isdefined(Main, :TestOptions) ? Main.TestOptions.VERBOSE : true const SHOWTIMING = isdefined(Main, :TestOptions) ? Main.TestOptions.SHOWTIMING : true @@ -255,9 +251,11 @@ function test_canonical() passed_tests = passed_tests_ref[] # ---------------------------------------------------------------- - # GPU tests (only if CUDA is available) + # GPU tests (device tier — only with a functional CUDA device). + # A lost device on a GPU runner is caught centrally by + # test/suite/environment/test_environment_contract.jl, not here. # ---------------------------------------------------------------- - if is_cuda_on() + if Main.TestCapabilities.CUDA_FUNCTIONAL # Define GPU modelers and solvers as lists (even with single element) gpu_modelers = [( "Exa", OptimalControl.Exa{OptimalControl.GPU}(backend=CUDA.CUDABackend())