get_differential_vars: check Diagonal.diag, not the full matrix (fixes GPU DAE 100% failure)#3923
Open
singhharsh1708 wants to merge 1 commit into
Conversation
Member
|
The GPU job on this PR gets past the This appears to confirm that this PR cleared the earlier scalar-indexing barrier, after which CI reached the unrelated QR barrier. I would keep this PR focused; its GPU job cannot become green until the CUDA fix is available in the resolved environment. |
singhharsh1708
force-pushed
the
fix-gpu-diagonal-massmatrix-scalar-index
branch
from
July 18, 2026 07:32
8281897 to
ff724e9
Compare
…L#3922) all(!iszero, mm) visits every (i, j) pair, including scalar getindex on the underlying diagonal vector. For a GPU-backed diagonal mass matrix (Diagonal{Float64, <:CuVector}), that's disallowed and throws on every DAE init, which is why every case in test/gpu/dae_tests.jl fails regardless of solver or jac_prototype. Diagonal structurally guarantees off-diagonal zeros, so check mm.diag directly. 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.
singhharsh1708
force-pushed
the
fix-gpu-diagonal-massmatrix-scalar-index
branch
from
July 21, 2026 14:31
ff724e9 to
59b1424
Compare
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.
Fixes #3922.
Bug
get_differential_vars(lib/OrdinaryDiffEqCore/src/misc_utils.jl) checks the mass matrix with:before the
_isdiagbranch. For aDiagonal,all(!iszero, ::AbstractMatrix)visits every(i, j)pair, including a call togetindexon the underlying diagonal vector for eachi == j.test/gpu/dae_tests.jlbuilds its mass matrix ascu(Diagonal([1, 1, 0, 0])), i.e.Diagonal{Float64, <:CuVector}— CUDA disallows scalargetindexon device arrays by default, so this throws on every DAE init.get_differential_varsruns wheneverWhasn't already been built (lib/OrdinaryDiffEqCore/src/solve.jl:592), which is the common case, so this hits every solver × jac_prototype combination in that test file — matching the 100% failure rate on the GPU CI lane since it was turned back on in #3813.Fix
Diagonalstructurally guarantees off-diagonal zeros, so checkmm.diagdirectly instead of the full matrix. Same result for CPUDiagonal(verified against master: identical output for mixed/all-nonzero/all-zero diagonals), just without the wasted O(n²) structural traversal, and without the unsafe scalar indexing for a GPU-backed diagonal.Also wires up
show_errors(results)in the GPU DAE test's@testsetentry point. It was defined but never called, so CI logs only ever showed a status glyph, no exception text — that's why this took reading the full raw job log to pin down instead of the test's own diagnostics.Verification
No GPU hardware available here, so this isn't verified against real CUDA. What is verified:
get_differential_varsonDiagonalis byte-for-byte identical before/after the change (mixed, all-nonzero, all-zero diagonal cases).Sparse isdiag Performance(10/10) andAlgebraic Vars Detection(21/21, includes 3 new tests for theDiagonalcase) pass.The
mm.diagreduction is exactly the same operation CUDA.jl already supports forall/anyover aCuVector(proper GPU reduction, not scalar indexing), so this should clear the GPU failure, but someone with GPU access needs to confirm.AI Disclosure
Claude assisted with this work.