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

repeated application of structural_simplify leads to error #3012

Closed
hexaeder opened this issue Sep 4, 2024 · 1 comment · Fixed by #3027 or #3035
Closed

repeated application of structural_simplify leads to error #3012

hexaeder opened this issue Sep 4, 2024 · 1 comment · Fixed by #3027 or #3035
Labels
bug Something isn't working

Comments

@hexaeder
Copy link
Contributor

hexaeder commented Sep 4, 2024

Describe the bug 🐞

Repeated aplication of structural_simplify can lead to errors. I've observed at least two:

  • unbalanced system error and
  • (far less important) out of bounds error on equations in a case where structural simplify reduced the system to 0 equations

Expected behavior

Either error because it should not be called on a completed system or be a no-op.

Minimal Reproducible Example 👇

using Pkg
pkg"activate --temp"
pkg"add ModelingToolkit, ModelingToolkitStandardLibrary"

using ModelingToolkit
using ModelingToolkitStandardLibrary.Electrical
using ModelingToolkitStandardLibrary.Blocks: Constant
using ModelingToolkit: t_nounits as t

R = 1.0
C = 1.0
V = 1.0
systems = @named begin
    resistor = Resistor(R = R)
    capacitor = Capacitor(C = C, v = 0.0)
    source = Voltage()
    constant = Constant(k = V)
    ground = Ground()
end

rc_eqs = [connect(constant.output, source.V)
          connect(source.p, resistor.p)
          connect(resistor.n, capacitor.p)
          connect(capacitor.n, source.n, ground.g)]

@named rc_model = ODESystem(rc_eqs, t; systems)
sys = structural_simplify(rc_model)
structural_simplify(sys)

Error & Stacktrace ⚠️

julia> structural_simplify(sys)
ERROR: ExtraVariablesSystemException: The system is unbalanced. There are 2 highest order derivative variables and 1 equations.
More variables than equations, here are the potential extra variable(s):
 capacitor₊v(t)
 capacitor₊i(t)
Note that the process of determining extra variables is a best-effort heuristic. The true extra variables are dependent on the model and may not be in this list.
Stacktrace:
  [1] error_reporting(state::TearingState{…}, bad_idxs::Vector{…}, n_highest_vars::Int64, iseqs::Bool, orig_inputs::Set{…})
    @ ModelingToolkit.StructuralTransformations ~/.julia/packages/ModelingToolkit/yfT8s/src/structural_transformation/utils.jl:50
  [2] check_consistency(state::TearingState{ODESystem}, orig_inputs::Set{Any})
    @ ModelingToolkit.StructuralTransformations ~/.julia/packages/ModelingToolkit/yfT8s/src/structural_transformation/utils.jl:85
  [3] _structural_simplify!(state::TearingState{…}, io::Nothing; simplify::Bool, check_consistency::Bool, fully_determined::Bool, warn_initialize_determined::Bool, dummy_derivative::Bool, kwargs::@Kwargs{})
    @ ModelingToolkit ~/.julia/packages/ModelingToolkit/yfT8s/src/systems/systemstructure.jl:692
  [4] _structural_simplify!
    @ ~/.julia/packages/ModelingToolkit/yfT8s/src/systems/systemstructure.jl:675 [inlined]
  [5] structural_simplify!(state::TearingState{…}, io::Nothing; simplify::Bool, check_consistency::Bool, fully_determined::Bool, warn_initialize_determined::Bool, kwargs::@Kwargs{})
    @ ModelingToolkit ~/.julia/packages/ModelingToolkit/yfT8s/src/systems/systemstructure.jl:635
  [6] __structural_simplify(sys::ODESystem, io::Nothing; simplify::Bool, kwargs::@Kwargs{})
    @ ModelingToolkit ~/.julia/packages/ModelingToolkit/yfT8s/src/systems/systems.jl:85
  [7] __structural_simplify
    @ ~/.julia/packages/ModelingToolkit/yfT8s/src/systems/systems.jl:66 [inlined]
  [8] structural_simplify(sys::ODESystem, io::Nothing; simplify::Bool, split::Bool, allow_symbolic::Bool, allow_parameter::Bool, conservative::Bool, fully_determined::Bool, kwargs::@Kwargs{})
    @ ModelingToolkit ~/.julia/packages/ModelingToolkit/yfT8s/src/systems/systems.jl:24
  [9] structural_simplify
    @ ~/.julia/packages/ModelingToolkit/yfT8s/src/systems/systems.jl:20 [inlined]
 [10] structural_simplify(sys::ODESystem)
    @ ModelingToolkit ~/.julia/packages/ModelingToolkit/yfT8s/src/systems/systems.jl:20
 [11] top-level scope
    @ REPL[40]:1
 [12] top-level scope
    @ none:1
Some type information was truncated. Use `show(err)` to see complete types.

Environment (please complete the following information):

  • Output of using Pkg; Pkg.status()
(jl_S36F9c) pkg> st
Status `/tmp/jl_S36F9c/Project.toml`
  [961ee093] ModelingToolkit v9.36.0
  [16a59e39] ModelingToolkitStandardLibrary v2.11.0
  • Output of versioninfo()
julia> versioninfo()
Julia Version 1.11.0-rc3
Commit 616e45539db (2024-08-26 15:46 UTC)
Build Info:
  Official https://julialang.org/ release
Platform Info:
  OS: Linux (x86_64-linux-gnu)
  CPU: 16 × AMD Ryzen 9 8945HS w/ Radeon 780M Graphics
  WORD_SIZE: 64
  LLVM: libLLVM-16.0.6 (ORCJIT, znver3)
Threads: 16 default, 0 interactive, 8 GC (on 16 virtual cores)
Environment:
  JULIA_NUM_THREADS = auto
@hexaeder hexaeder added the bug Something isn't working label Sep 4, 2024
@ChrisRackauckas
Copy link
Member

Either error because it should not be called on a completed system or be a no-op.

It shoulderror, and throw a nice error. Right now it throws a bad error 😅 but we should throw a nice contextual error if you try to repeat or call on a complete system.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants