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

Added: support for custom economic constraints gE in NonLinMPC #118

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

franckgaga
Copy link
Member

No description provided.

@codecov-commenter
Copy link

codecov-commenter commented Oct 9, 2024

Codecov Report

Attention: Patch coverage is 88.88889% with 1 line in your changes missing coverage. Please review.

Project coverage is 98.93%. Comparing base (3833301) to head (f0a8500).

Files with missing lines Patch % Lines
src/controller/nonlinmpc.jl 87.50% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #118   +/-   ##
=======================================
  Coverage   98.93%   98.93%           
=======================================
  Files          24       24           
  Lines        3575     3575           
=======================================
  Hits         3537     3537           
  Misses         38       38           

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

@baggepinnen
Copy link
Member

just a little note, I've never really come across the term "economic constraints" before, is this from some particular book on nonlinear MPC? I use nonlinear MPC with nonlinear constraints very often, but it almost never have an economic interpretation. Would it perhaps be more generic to call them "nonlinear constraints"? I'm thinking that someone who want's to add safety constraints such as collision avoidance will not find what they are looking for, not knowing to look for "economic constraints".

@franckgaga
Copy link
Member Author

franckgaga commented Oct 10, 2024

That's a good point. I named them economic constraints merely for consistency with my current notation. The nonlinear constraint function will not receive $\mathbf{U}$, $\mathbf{\hat{Y}}$ and $\mathbf{\hat{D}}$ but the slightly longer version appended with the $E$ subscript:

image

It is also incidently consistent with the custom objective term $J_E$.

Would it perhaps be more generic to call them "nonlinear constraints"?

It would not be precise enough since there is also the plant output and the terminal bounds that are also nonlinear (if the plant model is nonlinear). It's not a replacement of the nonlinear constraints but an addition to the current nonlinear constraints. Maybe just naming them "custom nonlinear constraints" would be sufficient. I would also change my notation to:

$$J_E(\mathbf{\bar{U}, \bar{Y}, \bar{D}, p}) \quad \text{and} \quad \mathbf{g_c}(\mathbf{\bar{U}, \bar{Y}, \bar{D}, \epsilon, p})$$

with the new notation $\mathbf{\bar{U}} = \mathbf{U}_E$, $\mathbf{\bar{Y}} = \mathbf{\hat{Y}}_E$ and $\mathbf{\bar{D}} = \mathbf{\hat{D}}_E$.

While we are at it, do you think that it's a bad idea to name the custom objective term the economic term? (but renaming it and changing the notation would be a breaking change since they are keyword arguments.)

@baggepinnen
Copy link
Member

While we are at it, do you think that it's a bad idea to name the custom objective term the economic term?

Not necessarily bad, but I'd imagine that many aren't familiar with the term.

but renaming it and changing the notation would be a breaking change since they are keyword arguments.

One way of changing stuff like this in a non-breaking way is to introduce the new keyword, keep the old, and then do something like

function fun(; new = default, old = nothing)
    new = something(old, new)
end

the function something returns the first argument that is not nothing. If the old keyword is removed from the docs, no one will use it in the future, but existing usage will continue to work.

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.

3 participants