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

Simulator does not allow non-zero D matrix #397

Closed
albheim opened this issue Nov 19, 2020 · 9 comments
Closed

Simulator does not allow non-zero D matrix #397

albheim opened this issue Nov 19, 2020 · 9 comments

Comments

@albheim
Copy link
Member

albheim commented Nov 19, 2020

I noticed that when I create a continuous system with equal number of poles and zeros I can't use step since it is complaining that a non-zero D-matrix is not supported.

The error comes from simulator.jl here

function Simulator(P::AbstractStateSpace, u::F = (x,t) -> 0) where F
@assert iscontinuous(P) "Simulator only supports continuous-time system. See function `lsim` for simulation of discrete-time systems."
@assert all(P.D .== 0) "Can not simulate systems with direct term D != 0"
f = (dx,x,p,t) -> dx .= P.A*x .+ P.B*u(x,t)
y(x,t) = P.C*x #.+ P.D*u(x,t)
y(sol::ODESolution,t) = P.C*sol(t)
Simulator(P, f, y)
end

Though it seems that in lsim, which is the function calling Simulator, it only expects the states to be calculated and calculates the output given the D-matrix by itself. Seems like these two are not entirely synced in how they work, Simulator exposes the y function which is not used and lsim seems like it does not have a problem with the D-matrix but Simulator has (which I still don't really see the reason why it is a problem).

s = Simulator(sys, u)
sol = solve(s, T.(x0), (t[1],t[end]), Tsit5())
x = sol(t)'
uout = Array{eltype(x)}(undef, length(t), ninputs(sys))
for (i,ti) in enumerate(t)
uout[i,:] = u(x[i,:],ti)'
end
end
y = transpose(sys.C*transpose(x) + sys.D*transpose(uout))
return y, t, x, uout

The assertion that D should be zero comes from a #118 in a commit named "Fix stack overflow error" by @baggepinnen, though when testing to remove that assertion it seems that running

using ControlSystems
sys = ss(-1, 2, 3, 4)
step(sys)

works fine and gives the values I expect when plotting. Running all tests also seems to work without the assertion. So I guess I'm wondering if @baggepinnen has any memory of why it was once needed and if it is still needed for some case that is not tested? Or could we just remove it?

@baggepinnen
Copy link
Member

I do not have any memory left of this unfortunately, but if you do not get said stack-overflow error I'm all for removing the restriction :)

@albheim
Copy link
Member Author

albheim commented Nov 19, 2020

Okay, will put together a PR for that then.

Also wondering about Simulator in general, is it used for any external things? It seems lsim is the only place we use it internally, and it seems lsim is only using parts of it (y function is ignored).

Was thinking that either it seems it could be dropped and lsim can contain those few extra rows, or maybe we should use the functionality exposed by Simulator and make sure the y function can be used properly with lsim?

@baggepinnen
Copy link
Member

There were once several different AbstractSimulator such as for gain scheduling and output feedback etc., but they have since been removed. Simulator still allows you to modify any part of the linear dynamics and thus simulate a party nonlinear system, e.g., Hammerstein/Weiner systems etc.
There are also some users of Simulators out there, but I couldn't find a discussion on it now so it has probably come up on slack :/

@olof3
Copy link
Contributor

olof3 commented Nov 19, 2020

The objective of Simulator is to enable simulation of feedback interconnections? To me it feels like a common task that it is very good to have support for doing in an easy way (including with nonlinear systems), but to me it seem like it could just be something like sim_feedback or something like that, and then you get all the signals out that you are interested in. What does the Simulator type add? Wouldn't it be straightforward to just have the gain-scheduling controller or the adaptive controller to be one of the nonlinear systems in Simulator or sim_feedback without the need to introduce other Simulator types?

@olof3 olof3 closed this as completed Nov 19, 2020
@olof3 olof3 reopened this Nov 19, 2020
@baggepinnen
Copy link
Member

I believe we could make good use of this
https://github.com/jonniedie/ComponentArrays.jl/blob/master/docs/src/examples/adaptive_control.md
I've used ComponentArrays quite a lot for simulations an optimization lately and it helps keeping the code sane. @jonniedie mentioned in https://discourse.julialang.org/t/adaptive-control-and-online-parameter-estimation/42054 that functionality for this might appear in a package one day. Such a package could perhaps replace the Simulator type and it's functionality?

@mfalt
Copy link
Member

mfalt commented Nov 20, 2020

I think that we should either make a small change in #398, like adding a kwarg to disable the check for D!=0, or make some major change to the Simulator interface.
If we want to remove Simulator completely, we should probably just deprecate it first.

@jonniedie
Copy link
Contributor

A few things stalled out with that package because I wanted to include a better interface for discrete/continuous hybrid systems, but I never landed on anything I particularly liked. I’ll be taking some time off work soon, though, so hopefully I’ll finally be able to get around to putting everything together. If, when I do, anything looks like it would be a good fit to live inside ControlSystems.jl instead of a separate package, let me know and I’ll submit a PR.

@baggepinnen
Copy link
Member

@albheim this can be closed now?

@albheim
Copy link
Member Author

albheim commented Jan 17, 2021

@albheim this can be closed now?

The initial question is resolved.

Then there are the questions that came up in the discussion about what to do about the Simulator type, but that is maybe better to continue in its own thread.

@albheim albheim closed this as completed Jan 17, 2021
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

No branches or pull requests

5 participants