fix: infer oop form for SDEProblem/SDEFunction with StaticArrays

[AayushSabharwal]

Close #2814

Checklist

• Appropriate tests were added
• Any code changes were done in a way that does not break public API
• All documentation related to code changes were updated
• The new code follows the
  contributor guidelines, in particular the SciML Style Guide and
  COLPRAC.
• Any new documentation only uses public API

Additional context

Add any other context about the problem here.

[AayushSabharwal]

The problem here is that this line doesn't work with static arrays. K is an SVector{1} whereas L is an SMatrix{1, 1} (since noise is a matrix). I guess that line should have an @..?

julia> [1;;] + [1]
1×1 Matrix{Int64}:
 2
julia> @SArray[1;;] + @SArray[1]
ERROR: DimensionMismatch: Sizes (Size(1, 1), Size(1,)) of input arrays do not match

[ChrisRackauckas]

That's a method only for diagonal noise, so that method won't work on array

[AayushSabharwal]

1x1 noise is diagonal, right? Which I guess is why I guess it hits that

[ChrisRackauckas]

Yeah, what we should do is detect that if isdiagonal then turn it it into a vector so it represents diagonal noise.

[TorkelE]

This update causes Catalyst CI to fail. Given that Catalyst has a very extensive test set, and often has catched things that the MTK tests have missed, would it make sense to not merge things which cause Catalyst downstream errors?

[TorkelE]

(I have modified Catalyst so that it works with this new version. However, generally, I do think we should try and not ignore the CI that we got)

[ChrisRackauckas]

I don't think the Catalyst tests are right here?

[TorkelE]

The Catalyst tests are fine. Basically, this PR goes from requiring an internal matrix to be Matrix{Num} from Matrix{Any} (I know Sam has argued for the latter representation being used, personally I have no strong opinion). Anyway, this breaks a couple of things (which I Catalyst I switched around so that it should be fine).

[ChrisRackauckas]

Oh I thought the Catalyst issue was the sizing thing we were talking about? Requiring Matrix{Num} isn't intentional, though it is somewhat bettter.

[TorkelE]

No, the sizing got fixed (thanks for that!). Practically, I'm not sure of changing to Matrix{Num} is a big change, but next time it would be useful to at least have a note that there has been an update which breaks the Catalyst tests 👍

[isaacsas]

Shouldn’t one be able to generate SDESystems without using Nums? If not, wouldn’t this require all compositional modeling tools to know to rewrap internal unwrapped expressions when generating SDESystems from other systems?

[isaacsas]

@ChrisRackauckas @AayushSabharwal I don't think this is correct code for symbolics. The problem is that isdiag calls iszero which for a non-Num will create a symbolic boolean expression, i.e.

julia> @variables A
e1-element Vector{Num}:
 A

julia> eq = 5*A^2 + 2*A + 65
65 + 2A + 5(A^2)

julia> iszero(eq)
false

julia> iszero(ModelingToolkit.unwrap(eq))
(65 + 2A + 5(A^2)) == 0

So this now explicitly breaks creating an SDESystem with unwrapped expressions.

[isaacsas]

Also, as @TorkelE mentioned Catalyst master tests should now generally pass, so if Catalyst tests fail when running downstream tests on an MTK PR that is meaningful again (unlike before we updated Catalyst to MTK 9).

[ChrisRackauckas]

The Catalyst v14 release hasn't been made yet, so it's still not testing the updated code?

[ChrisRackauckas]

The problem is that isdiag calls iszero which for a non-Num will create a symbolic boolean expression, i.e.

Thanks, we need to have a different isdiag that uses isequal. @AayushSabharwal can you prioritize this?

[ChrisRackauckas]

@isaacsas is there an issue open on this? If so, let's assign @AayushSabharwal to it.

[isaacsas]

I opened #2868

[isaacsas]

The Catalyst v14 release hasn't been made yet, so it's still not testing the updated code?

Also, doesn't this:

https://github.com/SciML/ModelingToolkit.jl/actions/runs/9772401615/job/26976712250#step:6:579

indicate it was tested against Catalyst master?