Skip to content

Run CheckInit after Initialization and Fix Core1 tests #1193

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 19 commits into from
May 6, 2025

Conversation

DhairyaLGandhi
Copy link
Member

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

This dispatch does need to be removed, but check CI for failures.

Add any other context about the problem here.

@DhairyaLGandhi DhairyaLGandhi changed the title Remove literal_getproperty(sol, Val{:u}) Run CheckInit after Initialization and Fix Core1 tests May 6, 2025
@DhairyaLGandhi DhairyaLGandhi marked this pull request as ready for review May 6, 2025 06:04
@DhairyaLGandhi
Copy link
Member Author

I think we need to provide w2 => -1. in u0_correct if we want to still check with CheckInit which will cause the initialization to be over determined.

I have marked a copy of the prob such that we have a gt case which has this over determined initialization but does not affect the other test cases which will want to run initialization with a fully determined system specifcally.

Further, there is a bug in the Rodas methods it seems like. The case with the fully determined initialization when solved with NoInit should error since it does not have a value or guess for the algebraic variable. Instead, it produces:

julia> new_sol = solve(prob_correctu0, Rodas5P(); initializealg = NoInit(), sensealg, abstol = 1e-6, reltol = 1e-3)
retcode: Success
Interpolation: specialized 4rd order "free" stiffness-aware interpolation
t: 10-element Vector{Float64}:
   0.0
   1.0e-6
   1.1e-5
   0.00011099999999999999
   0.0011109999999999998
   0.011110999999999996
   0.11111099999999996
   1.1111109999999995
  11.111110999999994
 100.0
u: 10-element Vector{Vector{Float64}}:
 [2.0, 0.0, 0.0, 1.0, 0.0]
 [2.0, 0.0, 0.0, 1.0, 0.0]
 [2.0, 0.0, 0.0, 1.0, 0.0]
 [2.0, 0.0, 0.0, 1.0, 0.0]
 [2.0, 0.0, 0.0, 1.0, 0.0]
 [2.0, 0.0, 0.0, 1.0, 0.0]
 [2.0, 0.0, 0.0, 1.0, 0.0]
 [2.0, 0.0, 0.0, 1.0, 0.0]
 [2.0, 0.0, 0.0, 1.0, 0.0]
 [2.0, 0.0, 0.0, 1.0, 0.0]

@ChrisRackauckas
Copy link
Member

I think we need to provide w2 => -1. in u0_correct if we want to still check with CheckInit which will cause the initialization to be over determined.

Not true, if there's a guess that should be used, unless there's still a bug there @AayushSabharwal

@ChrisRackauckas
Copy link
Member

Further, there is a bug in the Rodas methods it seems like. The case with the fully determined initialization when solved with NoInit should error since it does not have a value or guess for the algebraic variable. Instead, it produces:

No, that's why I was saying you shouldn't use NoInit here.

@ChrisRackauckas
Copy link
Member

I just checked. Guess propagation is working fine on my end:

using OrdinaryDiffEq, ModelingToolkit
using ModelingToolkit: t_nounits as t, D_nounits as D
@parameters σ ρ β A[1:3]
@variables x(t) y(t) z(t) w(t) w2(t)

eqs = [D(D(x)) ~ σ * (y - x),
    D(y) ~ x *- z) - y,
    D(z) ~ x * y - β * z,
    w ~ x + y + z + 2 * β,
    0 ~ x^2 + y^2 - w2^2
]

@mtkbuild sys = ODESystem(eqs, t)

u0_incorrect = [D(x) => 2.0,
    x => 1.0,
    y => 0.0,
    z => 0.0]

p ==> 28.0,
    ρ => 10.0,
    β => 8 / 3]

tspan = (0.0, 100.0)

u0_correct = [D(x) => 2.0,
    x => 1.0,
    y => 0.0,
    z => 0.0,]
prob_correctu0 = ODEProblem(sys, u0_correct, tspan, p, jac = true, guesses = [w2 => -1.0])
mtkparams_correctu0 = SciMLSensitivity.parameter_values(prob_correctu0)
test_sol = solve(prob_correctu0, Rodas5P(), initializealg = CheckInit(), abstol = 1e-6, reltol = 1e-3)

@ChrisRackauckas
Copy link
Member

julia> test_sol = solve(prob_correctu0, Rodas5P(), initializealg = CheckInit(), abstol = 1e-6, reltol = 1e-3)
retcode: Success
Interpolation: specialized 4rd order "free" stiffness-aware interpolation
t: 10232-element Vector{Float64}:
   0.0
   1.0e-6
   4.6676023541089825e-6
   1.406277673596804e-5
   3.278144741397908e-5
   6.419148942107095e-5
   0.00011116456475568178
   0.00017786678081930103
   0.000271133385736624
   ⋮
  99.91687143040015
  99.92926704759054
  99.94166266478094
  99.95539352281644
  99.9646987791528
  99.97400403548916
  99.98330929182552
  99.99261454816188
 100.0
u: 10232-element Vector{Vector{Float64}}:
 [2.0, 0.0, 0.0, 1.0, -1.0]
 [1.9999720001120003, 1.0000004999950436e-5, 5.000003607056631e-12, 1.000001999986, -1.000002000036]
 [1.9998693095741922, 4.6676132468589305e-5, 1.0893293993865827e-10, 1.0000093348997008, -1.0000093359890214]
 [1.9996062644011885, 0.00014062875602951922, 9.888190132121024e-10, 1.0000281227849122, -1.0000281326728617]
 [1.9990822398364054, 0.00032781984549872586, 5.3732513186776715e-9, 1.0000655478514173, -1.0000656015808531]
 [1.9982030998441016, 0.0006419354837430721, 2.0603753564580495e-8, 1.0001283253010556, -1.000128531315339]
 [1.9968887764760062, 0.0011117073667597829, 6.179309726719579e-8, 1.0002221561749574, -1.0002227739846816]
 [1.9950232744291352, 0.0017788257101189419, 1.5820467061650703e-7, 1.0003552908594773, -1.0003568724082876]
 [1.9924165022118636, 0.0027117004285139934, 3.6764346859572156e-7, 1.0005412383294567, -1.0005449129975832]
 ⋮
 [22.451502965977113, 3.5763156602418764, 14.49076032504619, 19.095009873023972, -19.42720577938862]
 [16.825095478698696, 2.4355128410705102, 14.724562457922266, 19.338935013763827, -19.49181125475473]
 [10.72457093433667, 1.2739062770137506, 14.684097844197916, 19.510161047524054, -19.55174035252264]
 [3.4519383074370587, 0.0475040376449663, 14.326813158830632, 19.608073663940203, -19.608065632865248]
 [-1.7464560579732538, -0.7034545257092123, 13.914876060126806, 19.616162583416656, -19.628713340973025]
 [-7.124655444976655, -1.3610934986046739, 13.38588197040998, 19.57501386541934, -19.622205586841815]
 [-12.642627570620036, -1.9057588504443905, 12.762322984740244, 19.48313632696152, -19.576047368186142]
 [-18.255577651189146, -2.323296528666386, 12.06983231985069, 19.339434641524246, -19.47841981533405]
 [-22.746257645129607, -2.558672337734426, 11.489307512570512, 19.188037129458547, -19.357853875055586]

@DhairyaLGandhi
Copy link
Member Author

Core 8 also has a new failure with

  LoadError: Cyclic guesses detected in the system. Symbolic values were found for the following variables/parameters in the map: 
  ampermeter₊n₊v(t)  => capacitor2₊v(t) + input_signal₊output₊u(t)
  In order to resolve this, please provide additional numeric guesses so that the chain can be resolved to assign numeric values to each variable.

Should we add a guess here?

Co-authored-by: Aayush Sabharwal <[email protected]>
@ChrisRackauckas
Copy link
Member

🎉

@ChrisRackauckas ChrisRackauckas merged commit afaf14a into master May 6, 2025
24 of 42 checks passed
@ChrisRackauckas ChrisRackauckas deleted the dg/core1_init branch May 6, 2025 23:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants