Skip to content

Conversation

@franckgaga
Copy link
Member

@franckgaga franckgaga commented May 7, 2025

Include calling rethrow() instead of rethrow(err) and minor corrections in doc and tests

@codecov-commenter
Copy link

codecov-commenter commented May 7, 2025

Codecov Report

Attention: Patch coverage is 83.33333% with 4 lines in your changes missing coverage. Please review.

Project coverage is 98.81%. Comparing base (6c5f770) to head (128922f).
Report is 17 commits behind head on main.

Files with missing lines Patch % Lines
src/controller/execute.jl 33.33% 2 Missing ⚠️
src/estimator/mhe/execute.jl 83.33% 1 Missing ⚠️
src/general.jl 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #200      +/-   ##
==========================================
- Coverage   98.90%   98.81%   -0.10%     
==========================================
  Files          25       25              
  Lines        4220     4223       +3     
==========================================
- Hits         4174     4173       -1     
- Misses         46       50       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@franckgaga
Copy link
Member Author

The MTK example in the manual no longer work. For now, I will pin MTK version in the doc environment and add compat admonition in the doc.

@baggepinnen or @1-Bart-1 are you aware of any change in MTK recently that broke the MTK code ?

The model is:

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)

it crashes at this line:

    (_, f_ip), dvs, psym, io_sys = ModelingToolkit.generate_control_function(
        model, inputs, split=false; outputs
    )

the error is:

ERROR: The LHS cannot contain nondifferentiated variables. Please run `structural_simplify` or use the DAE form.
Got y(t) ~ 57.29577951308232θ(t)
Stacktrace:
 [1] error(s::String)
   @ Base ./error.jl:35
 [2] check_operator_variables(eqs::Vector{Equation}, op::Type{Differential})
   @ ModelingToolkit ~/.julia/packages/ModelingToolkit/Iruox/src/utils.jl:361
 [3] generate_control_function(sys::ODESystem, inputs::Vector{…}, disturbance_inputs::Vector{…}; disturbance_argument::Bool, implicit_dae::Bool, simplify::Bool, eval_expression::Bool, eval_module::Module, kwargs::@Kwargs{})
   @ ModelingToolkit ~/.julia/packages/ModelingToolkit/Iruox/src/inputoutput.jl:233
 [4] generate_f_h(model::ODESystem, inputs::Vector{Num}, outputs::Vector{Num})
   @ Main ~/.julia/dev/ModelPredictiveControl/docs/src/manual/mtk.md:60
 [5] top-level scope
   @ ~/.julia/dev/ModelPredictiveControl/docs/src/manual/mtk.md:105
Some type information was truncated. Use `show(err)` to see complete types.

Once again, a not-so-helpful error message since it is clearly mentionned in generate_control_function docstring that we should not call structural_simplify

@franckgaga
Copy link
Member Author

FYI, the exemple work in MTK v9.76.0. So the breaking change is in v9.77.0

@franckgaga franckgaga merged commit 5c85eeb into main May 7, 2025
4 checks passed
@franckgaga franckgaga deleted the cherry_pick branch May 7, 2025 20:35
@baggepinnen
Copy link
Member

I'm not sure what has changed in MTK, when MTK examples break it's usually only @AayushSabharwal who knows why

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.

4 participants