Skip to content

Commit e06103c

Browse files
Fix jacobian2W! DimensionMismatch for ScalarOperator mass matrix
ScalarOperator reports axes(mm) == (), unlike UniformScaling which jacobian2W! already special-cases, so it fell into the boundscheck meant for full matrices. Fixes #3915.
1 parent 30fc999 commit e06103c

4 files changed

Lines changed: 43 additions & 10 deletions

File tree

lib/OrdinaryDiffEqDifferentiation/src/OrdinaryDiffEqDifferentiation.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import StaticArraysCore: StaticArray, StaticMatrix
2323
using SciMLBase: UJacobianWrapper, UDerivativeWrapper, _vec, _unwrap_val
2424
import SciMLBase: SciMLBase, @set, DEIntegrator, ODEFunction, SplitFunction, DAEFunction, remake, solve!
2525
import SciMLOperators: SciMLOperators, update_coefficients, update_coefficients!, MatrixOperator, AbstractSciMLOperator,
26-
islinear, isconstant
26+
islinear, isconstant, ScalarOperator
2727
import SparseMatrixColorings: ConstantColoringAlgorithm, GreedyColoringAlgorithm, ColoringProblem,
2828
ncolors, column_colors, coloring, sparsity_pattern
2929
import OrdinaryDiffEqCore

lib/OrdinaryDiffEqDifferentiation/src/derivative_utils.jl

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -578,6 +578,13 @@ function do_newJW(integrator, alg, nlsolver, repeat_step)::NTuple{2, Bool}
578578
end
579579
end
580580

581+
# `ScalarOperator` (λ·I) reports `axes(mm) == ()` like `UniformScaling`, but unlike
582+
# `UniformScaling` it isn't matched by `isa UniformScaling` -- treat both as the same
583+
# scalar-times-identity case rather than requiring `axes(mm) == axes(W)`.
584+
_is_scalar_massmatrix(mm) = mm isa UniformScaling || mm isa ScalarOperator
585+
_scalar_massmatrix_λ(mm::UniformScaling) = mm.λ
586+
_scalar_massmatrix_λ(mm::ScalarOperator) = mm.val
587+
581588
@noinline _throwWJerror(W, J) = throw(DimensionMismatch("W: $(axes(W)), J: $(axes(J))"))
582589
@noinline function _throwWMerror(W, mass_matrix)
583590
throw(DimensionMismatch("W: $(axes(W)), mass matrix: $(axes(mass_matrix))"))
@@ -608,14 +615,14 @@ function jacobian2W!(
608615
# check size and dimension
609616
iijj = axes(W)
610617
@boundscheck (iijj == axes(J) && length(iijj) == 2) || _throwWJerror(W, J)
611-
mass_matrix isa UniformScaling ||
618+
_is_scalar_massmatrix(mass_matrix) ||
612619
@boundscheck axes(mass_matrix) == axes(W) || _throwWMerror(W, mass_matrix)
613620
@inbounds begin
614621
invdtgamma = inv(dtgamma)
615-
if mass_matrix isa UniformScaling
622+
if _is_scalar_massmatrix(mass_matrix)
616623
copyto!(W, J)
617624
idxs = diagind(W)
618-
λ = -mass_matrix.λ
625+
λ = -_scalar_massmatrix_λ(mass_matrix)
619626
if ArrayInterface.fast_scalar_indexing(J) &&
620627
ArrayInterface.fast_scalar_indexing(W)
621628
@inbounds for i in 1:size(J, 1)
@@ -639,14 +646,14 @@ function jacobian2W!(W::Matrix, mass_matrix, dtgamma::Number, J::Matrix)::Nothin
639646
# check size and dimension
640647
iijj = axes(W)
641648
@boundscheck (iijj == axes(J) && length(iijj) == 2) || _throwWJerror(W, J)
642-
mass_matrix isa UniformScaling ||
649+
_is_scalar_massmatrix(mass_matrix) ||
643650
@boundscheck axes(mass_matrix) == axes(W) || _throwWMerror(W, mass_matrix)
644651
@inbounds begin
645652
invdtgamma = inv(dtgamma)
646-
if mass_matrix isa UniformScaling
653+
if _is_scalar_massmatrix(mass_matrix)
647654
copyto!(W, J)
648655
idxs = diagind(W)
649-
λ = -mass_matrix.λ
656+
λ = -_scalar_massmatrix_λ(mass_matrix)
650657
@inbounds for i in 1:size(J, 1)
651658
W[i, i] = muladd(λ, invdtgamma, J[i, i])
652659
end
@@ -661,12 +668,12 @@ end
661668

662669
function jacobian2W(mass_matrix, dtgamma::Number, J::AbstractMatrix)
663670
# check size and dimension
664-
mass_matrix isa UniformScaling ||
671+
_is_scalar_massmatrix(mass_matrix) ||
665672
@boundscheck axes(mass_matrix) == axes(J) || _throwJMerror(J, mass_matrix)
666673
@inbounds begin
667674
invdtgamma = inv(dtgamma)
668-
if mass_matrix isa UniformScaling
669-
λ = -mass_matrix.λ
675+
if _is_scalar_massmatrix(mass_matrix)
676+
λ = -_scalar_massmatrix_λ(mass_matrix)
670677
W = J +* invdtgamma) * I
671678
else
672679
W = muladd(-mass_matrix, invdtgamma, J)

lib/OrdinaryDiffEqDifferentiation/test/runtests.jl

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ end
3030
# Run functional tests
3131
if TEST_GROUP ("QA", "Sparse", "ModelingToolkit")
3232
@time @safetestset "DAE jacobian2W sparse" include("dae_jacobian2w_sparse_tests.jl")
33+
@time @safetestset "ScalarOperator mass matrix" include("scalar_operator_massmatrix_tests.jl")
3334
@time @safetestset "nzval helpers" include("nzval_helpers_tests.jl")
3435
@time @safetestset "prepare_sparse_jac!" include("prepare_sparse_jac_tests.jl")
3536
@time @safetestset "OOP J_t Tracking" include("oop_jt_tracking_test.jl")
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
using OrdinaryDiffEqDifferentiation
2+
using SciMLOperators
3+
using LinearAlgebra
4+
using Test
5+
6+
# ScalarOperator (λ·I) reports axes(mm) == (), unlike UniformScaling, so it fell
7+
# through the `mass_matrix isa UniformScaling` special case and hit the
8+
# `axes(mass_matrix) == axes(W)` boundscheck meant for full mass matrices.
9+
J = [1.0 2.0; 3.0 4.0]
10+
λ = 2.0
11+
W_expected = J - λ * inv(0.5) * I
12+
13+
W = similar(J)
14+
OrdinaryDiffEqDifferentiation.jacobian2W!(W, ScalarOperator(λ), 0.5, J)
15+
@test W W_expected
16+
17+
W_uniform = similar(J)
18+
OrdinaryDiffEqDifferentiation.jacobian2W!(W_uniform, λ * I, 0.5, J)
19+
@test W W_uniform
20+
21+
@test OrdinaryDiffEqDifferentiation.jacobian2W(ScalarOperator(λ), 0.5, J) W_expected
22+
23+
W_dense = Matrix(J)
24+
OrdinaryDiffEqDifferentiation.jacobian2W!(W_dense, ScalarOperator(λ), 0.5, Matrix(J))
25+
@test W_dense W_expected

0 commit comments

Comments
 (0)