Preparation for v1.0.0 release

[franckgaga]

I don't plan for any other breaking change in the next releases. I feel that most essential features are there, and the public interface is flexible enough to accommodate many types of predictive controllers and state estimators. I feel like its time for the 1.0.0 release.

Also good news: I re-benchmarked the case studies in the manuscript and there is improvements using the last release (presumably caused by all the allocation improvements, and maybe some improvements in Ipopt/JuMP side). The first case study median time was ~750 ms with Ipopt. It is now ~450 ms.

About that, do you think that we should update the results in the paper (if accepted) ? It means updating all the figures since the new default current estimators produces different closed-loop responses. It may be worth it since the performance is just better, both for the disturbance rejection and the computational time.

I will wait a little bit before merging into main. In the meantime I will improve coverage and debug if required.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 97.67442% with 2 lines in your changes missing coverage. Please review.

Project coverage is 98.96%. Comparing base (4dae85d) to head (9cc320b).
Report is 20 commits behind head on main.

Files with missing lines	Patch %	Lines
src/controller/nonlinmpc.jl	96.42%	1 Missing ⚠️
src/model/solver.jl	88.88%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #105      +/-   ##
==========================================
+ Coverage   96.95%   98.96%   +2.01%     
==========================================
  Files          24       24              
  Lines        3577     3574       -3     
==========================================
+ Hits         3468     3537      +69     
+ Misses        109       37      -72     

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

[baggepinnen]

The only breaking change I have had in mind, which I leave up to you to decide about, is the signature of the nonlinear dynamics function. Right now it is

pendulum(par, x, u)

but it could potentially be worthwhile changing this to follow the SciML and ControlSystems.jl convention

pendulum(x, u, par, t)

where par comes after x,u and t is additionally included. Right now, t isn't being used at all in this package, but there are cases where it's useful. A user of SciML can always manually write a wrapper function that changes the argument order, so it's a non-critical thing.

About that, do you think that we should update the results in the paper (if accepted) ? It means updating all the figures since the new default current estimators produces different closed-loop responses.

Sounds worth it to me, I guess you have some script that runs them all and outputs the figures?

[franckgaga]

Good idea! I think I'll switch the order.

Right now, model parameters are not officially supported. I just use closure when needed. It works well but it feels dangerous, for type stability. That's why I added a const in the manual.

[franckgaga]

Sounds worth it to me, I guess you have some script that runs them all and outputs the figures?

Yep, that's the same script that produce the figure et run the benchmarks. I may have a problem with KNITRO tho, I used the 6-months demo license and I think it's expired.

[franckgaga]

Right now, model parameters are not officially supported. I just use closure when needed. It works well but it feels dangerous, for type stability. That's why I added a const in the manual.

About that, it may be interesting to officially support a parameter argument in NonLinModel. How it's implemented in JuliaSimControl? The parameter arguments is a normal vector store in your struct ? Or it can be any object and its concrete type is handled through a struct with parametric composite types ?

[baggepinnen]

I just keep a

struct MPC{T}
  ...
  p::T
end

that can be of any type the user decides, it's just passed along to the dynamics function (and constraints etc. in our case).

[franckgaga]

The only breaking change I have had in mind, which I leave up to you to decide about, is the signature of the nonlinear dynamics function. Right now it is

pendulum(par, x, u)

but it could potentially be worthwhile changing this to follow the SciML and ControlSystems.jl convention

pendulum(x, u, par, t)

where par comes after x,u and t is additionally included. Right now, t isn't being used at all in this package, but there are cases where it's useful. A user of SciML can always manually write a wrapper function that changes the argument order, so it's a non-critical thing.

I forgot about the measured disturbance argument d. It will be impossible to perfectly match the signature of MTK because of this additional argument d. And I don't want to merge u and d into a single vector for the reasons explained in #75 (allow direct transmission with d only). Thus the signature will be:

pendulum(x, u, d, p, t)

[franckgaga]

@baggepinnen In your experience, did you encountered any control application in which the plant model ODE was a function of the time $t$, i.e. a non-autonomous dynamical system ? I personally never encountered them. It feels that it's very uncommon for control problems. I have LTV linear system in mind, but it's generally not directly a function of $t$, but rather an external condition that change the state-space matrices. This type of model is already handled with the setmodel! function on LinMPC. I know that there are some underground identification methods that return actual LTV model, it could be a use case...

Here are some arguments against adding $t$ in the signature:

1- It would be a bad idea remove the d argument, so a perfect match with MTK signature is impossible anyway
2- I looked at other mpc package (matlab, do-mpc, libmpc) and most of them do not include t in their signature
3- I need to add code that will keep track of the current discrete time-step $k$ in SimModel objects. It's super lightweight but I still feel that it's a bloat feature if nobody will use it.
4- All non-autonomous dynamical can be transformed into a autonomous system by adding a new measured disturbance $d(t)=t$.

Do you still think that adding $t$ is a good idea ? Quite frankly I'm opened to both options.

edit: I like the idea of HILO-MPC python package. There are 2 types of model: one for nonlinear time-invariant (TI), the other for nonlinear time-varying (TV) plant models. It could the solution : NonLinModel would be TI and I could add a new type later for nonlinear TV models.

[baggepinnen]

The fact that d is an option can in many cases remove the need to have t available. Most sources of time dependence I encounter are due to some form of external factor varying with time, e.g.,

• Disturbance variance may be higher at some points during the day than others. For example, wind speeds are often both higher on average and have a larger larger variation during day time than night time.
• The cost function might vary with time, e.g., the way electrical power tariffs are computed vary depending on the time of day and day of week in some places.
• Constraints may vary with time, e.g., you don't care as much about the temperature in an office building during night as you do during the day.

So to summarize, it's not always the dynamics that depend on time, it might be cost, constraints, noise etc.

[franckgaga]

Interesting examples, thanks for all these great advices Fredrik!

The other issue (minor TBF) with adding a $t$ argument is that it needs an implicit definition of what is $t = 0$ . The obvious solution would be to define $t = 0$ just after model/controller construction, or just after calling initstate!. That's what I implemented yesterday.

But your example with Hydro bill (I means electricity tariffs, canadian stuff 😝 ) is great since it need a more precise definition of what is $t = 0$ . On the other hand, letting the user explicitly handle that with the $d ( t ) = t$ trick remove this implicit definition. And, as you mentioned, the time-dependence can appear in the objective function. The measured disturbances are already an argument of the custom economical cost funcion $J_E$ in the package. So no modification on that part.

I will not add the $t$ argument. Explicitness is better than implicitness. I will just add this sentence in NonLinModel doc: If the dynamics is a function of time, simply add a measured disturbance defined as $d ( t ) = t$ .

A custom constraint function is not supported in the package for now but its in my TODO list.

[franckgaga]

https://juliacontrol.github.io/ModelPredictiveControl.jl/previews/PR105

@baggepinnen The documenentation of the PR is still building. I added the ModelingToolkit example in the manual. Does it seems ok for you ? I do not use the new model parameter feature since I'm not sure how to integrate that. It's not crucial at all, but do you have an idea how we would do that?

[franckgaga]

Weird.
The documentation build on my computer but not here. The line using ModelingToolkit: D_nounits as D, t_nounits as t, varmap_to_vars seems to be ignore since the error is :

UndefVarError: `D` not defined

I will investigate on this.

[franckgaga]

@JuliaRegistrator register

Release notes:

• breaking change: NonLinModel new p argument in f/f! and h/h! function signature
• breaking change: NonLinMPC new p argument in JE function signature
• added: support for plant model parameters $\mathbf{p}$ in NonLinModel
• added: support for economic function parameters $\mathbf{p}$ in NonLinMPC
• doc: ModelingToolkit integration example on the pendulum model
• test: improve coverage
• debug: ExplicitMPC correct error message with the unsupported setconstraint! method
• added: validate economic function $J_E$ argument signature

[franckgaga]

let's clean up the doc preview, maybe 🐈‍⬛