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

MTK example no longer work #116

Closed
franckgaga opened this issue Oct 7, 2024 · 18 comments · Fixed by #122
Closed

MTK example no longer work #116

franckgaga opened this issue Oct 7, 2024 · 18 comments · Fixed by #122

Comments

@franckgaga
Copy link
Member

franckgaga commented Oct 7, 2024

@baggepinnen The MTK example in the manual stop working and I'm not able to pinpoint why. It was working well when I released v1.0.1. It now crashes inside the h_ function (the function returned by ModelingToolkit.build_explicit_observed_function(io_sys, outputs; inputs) with the stacktrace:

ERROR: BoundsError: attempt to access Float64 at index [2]
Stacktrace:
  [1] getindex
    @ ./number.jl:98 [inlined]
  [2] macro expansion
    @ ~/.julia/packages/SymbolicUtils/ij6YM/src/code.jl:387 [inlined]
  [3] macro expansion
    @ ~/.julia/packages/RuntimeGeneratedFunctions/M9ZX8/src/RuntimeGeneratedFunctions.jl:163 [inlined]
  [4] macro expansion
    @ ./none:0 [inlined]
  [5] generated_callfunc
    @ ./none:0 [inlined]
  [6] (::RuntimeGeneratedFunctions.RuntimeGeneratedFunction{…})(::Vector{…}, ::Vector{…}, ::Vector{…}, ::Nothing)
    @ RuntimeGeneratedFunctions ~/.julia/packages/RuntimeGeneratedFunctions/M9ZX8/src/RuntimeGeneratedFunctions.jl:150
  [7] (::var"#h!#13"{})(y::Vector{…}, x::Vector{…}, ::Vector{…}, p::Vector{…})
    @ Main ~/.julia/dev/ModelPredictiveControl/docs/src/manual/mtk.md:86
  [8] h!
    @ ~/.julia/dev/ModelPredictiveControl/src/model/nonlinmodel.jl:211 [inlined]
  [9] evaloutput(model::NonLinModel{…}, d::Vector{…})
    @ ModelPredictiveControl ~/.julia/dev/ModelPredictiveControl/src/sim_model.jl:283
 [10] sim_closedloop!(est_mpc::NonLinMPC{…}, estim::UnscentedKalmanFilter{…}, N::Int64, u_ry::Vector{…}, d::Vector{…}, ru::Vector{…}; plant::NonLinModel{…}, u_step::Vector{…}, u_noise::Vector{…}, y_step::Vector{…}, y_noise::Vector{…}, d_step::Vector{…}, d_noise::Vector{…}, x_noise::Vector{…}, x_0::Vector{…}, xhat_0::Nothing, lastu::Vector{…}, x̂_0::Vector{…})
    @ ModelPredictiveControl ~/.julia/dev/ModelPredictiveControl/src/plot_sim.jl:282
 [11] #sim!#157
    @ ~/.julia/dev/ModelPredictiveControl/src/plot_sim.jl:244 [inlined]
 [12] sim!
    @ ~/.julia/dev/ModelPredictiveControl/src/plot_sim.jl:236 [inlined]
 [13] top-level scope
    @ ~/.julia/dev/ModelPredictiveControl/docs/src/manual/mtk.md:139
Some type information was truncated. Use `show(err)` to see complete types.

Do you have any idea why ?

edit: Also, what's your trick to debug these kind of error? I looked inside the lines at [1], [2], and so on in the stacktrace but the code is "hidden" because of the macros and stuff.

@franckgaga
Copy link
Member Author

@baggepinnen I just tested and the example work well with MTK v9.41. It stopped working with v9.42. Do you see anything v9.42 release that could have break this code ? Should I open an issue on MKT github ? For now I'll just restrict compat to 9.41 in the documentation Poject.toml.

image

@baggepinnen
Copy link
Member

@AayushSabharwal did anything change in parameter handling or otherwise for observed functions recently?

@AayushSabharwal
Copy link

Yes, but it shouldn't cause that error. If you give me something I can run, I'll pinpoint what broke

@baggepinnen
Copy link
Member

@franckgaga
Copy link
Member Author

franckgaga commented Oct 9, 2024

Here's a copy-paste of the code in the manual:

using ModelPredictiveControl, ModelingToolkit
using ModelingToolkit: D_nounits as D, t_nounits as t, varmap_to_vars
@mtkmodel Pendulum begin
    @parameters begin
        g = 9.8
        L = 0.4
        K = 1.2
        m = 0.3
    end
    @variables begin
        θ(t) # state
        ω(t) # state
        τ(t) # input
        y(t) # output
    end
    @equations begin
        D(θ)    ~ ω
        D(ω)    ~ -g/L*sin(θ) - K/m*ω + τ/m/L^2
        y       ~ θ * 180 / π
    end
end
@named mtk_model = Pendulum()
mtk_model = complete(mtk_model)

function generate_f_h(model, inputs, outputs)
    (_, f_ip), dvs, psym, io_sys = ModelingToolkit.generate_control_function(
        model, inputs, split=false; outputs
    )
    if any(ModelingToolkit.is_alg_equation, equations(io_sys))
        error("Systems with algebraic equations are not supported")
    end
    nu, nx, ny = length(inputs), length(dvs), length(outputs)
    vx = string.(dvs)
    p = varmap_to_vars(defaults(io_sys), psym)
    function f!(ẋ, x, u, _ , p)
        try
            f_ip(ẋ, x, u, p, nothing)
        catch err
            if err isa MethodError
                error("NonLinModel does not support a time argument t in the f function, "*
                      "see the constructor docstring for a workaround.")
            else
                rethrow()
            end
        end
        return nothing
    end
    h_ = ModelingToolkit.build_explicit_observed_function(io_sys, outputs; inputs)
    u_nothing = fill(nothing, nu)
    function h!(y, x, _ , p)
        y .= try
            # MTK.jl supports a `u` argument in `h_` function but not this package. We set
            # `u` as a vector of nothing and `h_` function will presumably throw an
            # MethodError it this argument is used inside the function
            h_(x, u_nothing, p, nothing)
        catch err
            if err isa MethodError
                error("NonLinModel only support strictly proper systems (no manipulated "*
                      "input argument u in the output function h)")
            else
                rethrow()
            end
        end
        return nothing
    end
    return f!, h!, p, nu, nx, ny, vx
end
inputs, outputs = [mtk_model.τ], [mtk_model.y]
f!, h!, p, nu, nx, ny, vx = generate_f_h(mtk_model, inputs, outputs)
Ts = 0.1
vu, vy = ["\$τ\$ (Nm)"], ["\$θ\$ (°)"]

model = setname!(NonLinModel(f!, h!, Ts, nu, nx, ny; p); u=vu, x=vx, y=vy)

mtk_model.K = defaults(mtk_model)[mtk_model.K] * 1.25
f_plant, h_plant, p = generate_f_h(mtk_model, inputs, outputs)
plant = setname!(NonLinModel(f_plant, h_plant, Ts, nu, nx, ny; p); u=vu, x=vx, y=vy)

α=0.01; σQ=[0.1, 1.0]; σR=[5.0]; nint_u=[1]; σQint_u=[0.1]
estim = UnscentedKalmanFilter(model; α, σQ, σR, nint_u, σQint_u)
Hp, Hc, Mwt, Nwt = 20, 2, [0.5], [2.5]
nmpc = NonLinMPC(estim; Hp, Hc, Mwt, Nwt, Cwt=Inf)
umin, umax = [-1.5], [+1.5]
nmpc = setconstraint!(nmpc; umin, umax)

using Plots
N = 35
res_ry = sim!(nmpc, N, [180.0], plant=plant, x_0=[0, 0], x̂_0=[0, 0, 0])
plot(res_ry)

@AayushSabharwal
Copy link

The observed function expects an MTKParameters, but the code uses a parameter vector

@baggepinnen
Copy link
Member

That looks like a bug in the observed function building then, the system io_sys was simplified using split = false?

@AayushSabharwal
Copy link

Right, I didn't notice the split = false. Weird.

@AayushSabharwal
Copy link

This is because prior to calling generate_control_function, there's this:

mtk_model = complete(mtk_model)

Which doesn't have split = false. complete does get called with split = false inside generate_control_function, but that just prevents it from doing @set! sys.index_cache = IndexCache(sys). It doesn't remove the IndexCache already created.

@AayushSabharwal
Copy link

I can make complete(sys; split = false) always remove the IndexCache, but it is worth noting that these two will not be identical:

sys1 = ODESystem(...)
sys2 = deepcopy(sys1)
sys1 = complete(sys1; split = false)
sys2 = complete(complete(sys2); split = false)

This is because complete with split = true reorders parameters.

@baggepinnen
Copy link
Member

I can make complete(sys; split = false) always remove the IndexCache

👍

but it is worth noting that these two will not be identical:

Is that likely to be a concern?

@AayushSabharwal
Copy link

Well a common workflow is parameters(sys) .=> values, which will break. Apart from that, as long as the only thing using the parameter order are the generated functions it shouldn't matter.

@baggepinnen
Copy link
Member

we take care in the example above to use the parameter order defined and returned by generate_control_function, psym. I hope that will be the final and correct order

@AayushSabharwal
Copy link

Yeah that should be fine

@franckgaga
Copy link
Member Author

@baggepinnen does that means the the problem should be solved on the latest stable MTK release ? (V9.49)

@baggepinnen
Copy link
Member

I just tested on MTK master and it worked. I think MTK might need a new release before it works, @AayushSabharwal?

@AayushSabharwal
Copy link

I'll tag MTK shortly, just waiting for CI to pass on master. It's currently queued

@franckgaga
Copy link
Member Author

I'll tag MTK shortly, just waiting for CI to pass on master. It's currently queued

Thanks @AayushSabharwal, the bugfixe works well!

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 a pull request may close this issue.

3 participants