Skip to content
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

Array parameters are not correctly scoped with ParentScope #2252

Closed
contradict opened this issue Sep 6, 2023 · 4 comments · Fixed by #2594 or #2603 · May be fixed by #2354
Closed

Array parameters are not correctly scoped with ParentScope #2252

contradict opened this issue Sep 6, 2023 · 4 comments · Fixed by #2594 or #2603 · May be fixed by #2354
Labels

Comments

@contradict
Copy link
Contributor

This used to work in ModelingToolkit 8.65.0 but stopped in 8.66.0.

@parameters a[1:2]=[1, 2] b=4 c=1
@variables t x(t)=ParentScope(a[1]) y(t)=ParentScope(b)
D = Differential(t)
@named level0 = ODESystem([D(x)~ParentScope(a[2]),
                           D(y)~ParentScope(c)], t, [x, y], [])
level1 = ODESystem(Equation[], t, [], [a..., b, c]; name=:level1) ∘ level0
level1 = structural_simplify(level1)
prob = ODEProblem(level1, [], (0, 1))

The first signs of trouble appear here, level0₊a[1] should not exist because of ParentScope, but the initial condition for y is scoped correctly.

julia> level1 = structural_simplify(level1)
Model level1 with 2 equations
States (2):
  level0₊x(t) [defaults to level0₊a[1]]
  level0₊y(t) [defaults to b]
Parameters (4):
  a[1] [defaults to 1]
  a[2] [defaults to 2]
  b [defaults to 4]
  c [defaults to 1]
Incidence matrix:2×4 SparseArrays.SparseMatrixCSC{Num, Int64} with 2 stored entries:
 ⋅  ⋅  ×  ⋅
 ⋅  ⋅  ⋅  ×

The equations show the same pattern

julia> equations(level1)
2-element Vector{Equation}:
 Differential(t)(level0₊x(t)) ~ level0₊a[2]
 Differential(t)(level0₊y(t)) ~ c

And constructing a problem fails.

julia> prob = ODEProblem(level1, [], (0, 1))
ERROR: ArgumentError: Any[level0₊a[1]] are either missing from the variable map or missing from the system's states/parameters list.
Stacktrace:
  [1] throw_missingvars_in_sys(vars::Vector{Any})
    @ ModelingToolkit ~/.julia/packages/ModelingToolkit/dkLCE/src/utils.jl:642
  [2] promote_to_concrete(vs::Vector{Any}; tofloat::Bool, use_union::Bool)
    @ ModelingToolkit ~/.julia/packages/ModelingToolkit/dkLCE/src/utils.jl:654
  [3] varmap_to_vars(varmap::Vector{Any}, varlist::Vector{Any}; defaults::Dict{Any, Any}, check::Bool, toterm::Function, promotetoconcrete::Nothing, tofloat::Bool, use_union::Bool)
    @ ModelingToolkit ~/.julia/packages/ModelingToolkit/dkLCE/src/variables.jl:92
  [4] get_u0_p(sys::ODESystem, u0map::Vector{Any}, parammap::SciMLBase.NullParameters; use_union::Bool, tofloat::Bool, symbolic_u0::Bool)
    @ ModelingToolkit ~/.julia/packages/ModelingToolkit/dkLCE/src/systems/diffeqs/abstractodesystem.jl:700
  [5] get_u0_p
    @ ~/.julia/packages/ModelingToolkit/dkLCE/src/systems/diffeqs/abstractodesystem.jl:684 [inlined]
  [6] process_DEProblem(constructor::Type, sys::ODESystem, u0map::Vector{Any}, parammap::SciMLBase.NullParameters; implicit_dae::Bool, du0map::Nothing, version::Nothing, tgrad::Bool, jac::Bool, checkbounds::Bool, sparse::Bool, simplify::Bool, linenumbers::Bool, parallel::Symbolics.SerialForm, eval_expression::Bool, use_union::Bool, tofloat::Bool, symbolic_u0::Bool, kwargs::Base.Pairs{Symbol, Integer, Tuple{Symbol, Symbol, Symbol}, NamedTuple{(:t, :has_difference, :check_length), Tuple{Int64, Bool, Bool}}})
    @ ModelingToolkit ~/.julia/packages/ModelingToolkit/dkLCE/src/systems/diffeqs/abstractodesystem.jl:724
  [7] (ODEProblem{true, SciMLBase.AutoSpecialize})(sys::ODESystem, u0map::Vector{Any}, tspan::Tuple{Int64, Int64}, parammap::SciMLBase.NullParameters; callback::Nothing, check_length::Bool, kwargs::Base.Pairs{Symbol, Union{}, Tuple{}, NamedTuple{(), Tuple{}}})
    @ ModelingToolkit ~/.julia/packages/ModelingToolkit/dkLCE/src/systems/diffeqs/abstractodesystem.jl:834
  [8] ODEProblem
    @ ~/.julia/packages/ModelingToolkit/dkLCE/src/systems/diffeqs/abstractodesystem.jl:827 [inlined]
  [9] (ODEProblem{true, SciMLBase.AutoSpecialize})(sys::ODESystem, u0map::Vector{Any}, tspan::Tuple{Int64, Int64})
    @ ModelingToolkit ~/.julia/packages/ModelingToolkit/dkLCE/src/systems/diffeqs/abstractodesystem.jl:827
 [10] (ODEProblem{true})(::ODESystem, ::Vector{Any}, ::Vararg{Any}; kwargs::Base.Pairs{Symbol, Union{}, Tuple{}, NamedTuple{(), Tuple{}}})
    @ ModelingToolkit ~/.julia/packages/ModelingToolkit/dkLCE/src/systems/diffeqs/abstractodesystem.jl:814
 [11] (ODEProblem{true})(::ODESystem, ::Vector{Any}, ::Vararg{Any})
    @ ModelingToolkit ~/.julia/packages/ModelingToolkit/dkLCE/src/systems/diffeqs/abstractodesystem.jl:813
 [12] ODEProblem(::ODESystem, ::Vector{Any}, ::Vararg{Any}; kwargs::Base.Pairs{Symbol, Union{}, Tuple{}, NamedTuple{(), Tuple{}}})
    @ ModelingToolkit ~/.julia/packages/ModelingToolkit/dkLCE/src/systems/diffeqs/abstractodesystem.jl:810
 [13] ODEProblem(::ODESystem, ::Vector{Any}, ::Vararg{Any})
    @ ModelingToolkit ~/.julia/packages/ModelingToolkit/dkLCE/src/systems/diffeqs/abstractodesystem.jl:809
 [14] top-level scope
    @ REPL[135]:1

@ChrisRackauckas
Copy link
Member

This is likely caused by #2247. I must've missed a case with handling array variables in the scoping recursion. I will likely take a look Saturday.

@contradict
Copy link
Contributor Author

I did some more poking at this one, I think the problem is actually in Symbolics. By the time namespace_expr is called, the SymScope metadata is gone. This code here rebuilds the indexed array from the array and the index, but the metadata was attached to the Term and is lost.

@contradict
Copy link
Contributor Author

@ChrisRackauckas I think I have fixed this with JuliaSymbolics/Symbolics.jl#1010 and #2352

@ChrisRackauckas
Copy link
Member

I think the solution generally looks fine, I just want to check with @YingboMa that the metadata for a scalar arrived from an array should propagate the same one, or if we want to amend it a bit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment