Skip to content

Commit aaef682

Browse files
get_differential_vars: check Diagonal.diag, not the full matrix
all(!iszero, mm) visits every (i, j) pair of the mass matrix via scalar getindex. For a GPU-backed diagonal mass matrix (Diagonal{Float64, <:CuVector}) that indexing is disallowed by default, so the check throws whenever it runs on a device array, and it is O(n^2) for a structurally O(n) question. Diagonal guarantees off-diagonal zeros, so check mm.diag directly; the broadcast over the diagonal vector dispatches to a device kernel, matching what find_algebraic_vars_eqs(::Diagonal) already does. Adds a guard test: a Diagonal wrapping a vector that errors on scalar getindex but supports whole-array broadcast, i.e. device array semantics on the CPU, so a regression back to the whole-matrix walk is caught without GPU hardware in CI. The statically resolvable Diagonal branch also fixes inference of solve for the two Rodas4 + ShampineCollocationInit cases in the NonlinearSolve mass matrix tests. Their form @test_broken sol = @inferred solve(...) errors with "expression evaluated to non-Boolean" once @inferred succeeds, so those two lines become @test (@inferred ...; true), still erroring if inference regresses. Also wires up show_errors(results) in the GPU DAE test entry point, which was defined but never called, so a real failure prints its actual exception instead of just a status glyph.
1 parent 2111422 commit aaef682

4 files changed

Lines changed: 50 additions & 2 deletions

File tree

lib/OrdinaryDiffEqCore/src/misc_utils.jl

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,12 @@ function get_differential_vars(f, u)
213213
elseif mm isa SciMLOperators.ScalarOperator
214214
# λ·I: every variable is algebraic when λ is zero.
215215
return iszero(mm.val) ? falses(size(u)) : nothing
216+
elseif mm isa Diagonal
217+
# Check only `mm.diag`: the generic paths below visit every (i, j)
218+
# pair via scalar `getindex`, which is disallowed when `mm.diag` is
219+
# GPU-backed (e.g. `Diagonal{Float64, <:CuVector}`), while a
220+
# broadcast over `mm.diag` dispatches to a proper GPU kernel.
221+
return reshape(mm.diag .!= 0, size(u))
216222
elseif all(!iszero, mm)
217223
return trues(size(mm, 1))
218224
elseif !(mm isa SciMLOperators.AbstractSciMLOperator) && _isdiag(mm)

lib/OrdinaryDiffEqCore/test/algebraic_vars_detection_tests.jl

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,3 +113,37 @@ end
113113
ff_singular = ODEFunction(f!, mass_matrix = ScalarOperator(0.0))
114114
@test get_differential_vars(ff_singular, u) == falses(3)
115115
end
116+
117+
@testset "get_differential_vars: Diagonal mass matrix" begin
118+
f!(du, u, p, t) = (du .= -u; nothing)
119+
u = zeros(4)
120+
ff_mixed = ODEFunction(f!, mass_matrix = Diagonal([1.0, 1.0, 0.0, 0.0]))
121+
@test get_differential_vars(ff_mixed, u) == Bool[1, 1, 0, 0]
122+
ff_alldiff = ODEFunction(f!, mass_matrix = Diagonal([1.0, 2.0, 3.0, 4.0]))
123+
@test get_differential_vars(ff_alldiff, u) == Bool[1, 1, 1, 1]
124+
ff_allalg = ODEFunction(f!, mass_matrix = Diagonal(zeros(4)))
125+
@test get_differential_vars(ff_allalg, u) == Bool[0, 0, 0, 0]
126+
end
127+
128+
# Mimics GPU device-array semantics on the CPU: elementwise scalar `getindex`
129+
# is forbidden (like CUDA's default `allowscalar(false)`), while whole-array
130+
# broadcast still works. Any code path that walks a `Diagonal` of this via
131+
# `getindex` -- `all(!iszero, mm)` or a `diag(mm)` copy -- errors, so the test
132+
# below catches a regression back to the whole-matrix check without needing a
133+
# GPU. Must be top-level: structs cannot be defined inside a `@testset`.
134+
struct NoScalarIndexVector{T} <: AbstractVector{T}
135+
data::Vector{T}
136+
end
137+
Base.size(v::NoScalarIndexVector) = size(v.data)
138+
function Base.getindex(::NoScalarIndexVector, ::Int)
139+
error("scalar getindex disallowed on NoScalarIndexVector")
140+
end
141+
Base.Broadcast.broadcastable(v::NoScalarIndexVector) = v.data
142+
143+
@testset "get_differential_vars: no scalar indexing of Diagonal mass matrix" begin
144+
f!(du, u, p, t) = (du .= -u; nothing)
145+
u = zeros(4)
146+
mm = Diagonal(NoScalarIndexVector([1.0, 1.0, 0.0, 0.0]))
147+
ff = ODEFunction(f!, mass_matrix = mm)
148+
@test get_differential_vars(ff, u) == Bool[1, 1, 0, 0]
149+
end

lib/OrdinaryDiffEqNonlinearSolve/test/mass_matrix_tests.jl

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -382,9 +382,12 @@ prob = ODEProblem(f, x0, tspan)
382382
foop = ODEFunction{false, SciMLBase.AutoSpecialize}(dynamics, mass_matrix = M)
383383
proboop = ODEProblem(f, x0, tspan)
384384
@test_broken sol = @inferred solve(prob, Rosenbrock23(autodiff = adalg))
385-
@test_broken sol = @inferred solve(prob, Rodas4(autodiff = adalg), initializealg = ShampineCollocationInit())
385+
# The Rodas4 + ShampineCollocationInit cases infer since get_differential_vars
386+
# got a statically resolvable Diagonal branch; keep @inferred so an inference
387+
# regression errors, but @test_broken would error ("non-Boolean") on success.
388+
@test (@inferred solve(prob, Rodas4(autodiff = adalg), initializealg = ShampineCollocationInit()); true)
386389
@test_broken sol = @inferred solve(proboop, Rodas5())
387-
@test_broken sol = @inferred solve(proboop, Rodas4(), initializealg = ShampineCollocationInit())
390+
@test (@inferred solve(proboop, Rodas4(), initializealg = ShampineCollocationInit()); true)
388391

389392
# InitialFailure with chunksize matching ODE size (> alg vars), #3157
390393
@testset "Mass Matrix: less alg vars than ODE AD chunksize" begin

test/gpu/dae_tests.jl

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -458,6 +458,11 @@ end
458458
@testset "GPU DAE solver compatibility" begin
459459
global results = run_all()
460460
show_results(results)
461+
# Print the actual caught exceptions on failure -- `show_results` only prints
462+
# a per-case status glyph, which is not enough on its own to diagnose what
463+
# broke from a CI log.
464+
any(r -> r.status (:gpu_error, :gpu_mismatch, :gpu_failed), results) &&
465+
show_errors(results)
461466
@testset "$(r.case)" for r in results
462467
# :cpu_skip = the CPU couldn't solve this case, so there is no reference
463468
# to compare against — record as skipped, not pass/fail.

0 commit comments

Comments
 (0)