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

lsim and lsimplot has problems with arguments u and x0 #460

Open
albheim opened this issue Mar 27, 2021 · 7 comments
Open

lsim and lsimplot has problems with arguments u and x0 #460

albheim opened this issue Mar 27, 2021 · 7 comments

Comments

@albheim
Copy link
Member

albheim commented Mar 27, 2021

See https://discourse.julialang.org/t/controlsystems-transfer-functions-with-delay/58003/7

  • Inconsistency between normal and delayed systems in how u is defined, u(x, t) vs u(t). Can easily lead to defining a zero input since delay system also accept u(uout, t) as an inplace version.
  • Missing documentation in the online docs about lsim for delayed systems, exists in help?> menu in repl
  • Does not handle different types for x0, for example if sys is float and x0 is int array
  • lsimplot can't handle x0 at all and gives strange error message
  • Not very clear what the states are, i.e. how the delay enters the system. This is important when setting the initial state x0. It is also not very clear how the delayed states are initialized and how you can affect them (not really possible as it is now but maybe it should be).
@albheim albheim changed the title lsim for normal and delayed use different u format lsim and lsimplot has problems with arguments u and x0 Mar 27, 2021
@albheim
Copy link
Member Author

albheim commented Mar 27, 2021

Maybe we should wait to fix this until #377 is finished so the fixes are applied to the future version of the delay systems, not sure if it really matters but it seems like the PR is pretty close so should not be too much work to finish it first.

@albheim
Copy link
Member Author

albheim commented Oct 7, 2021

The PR this was waiting for is now in, so maybe this discussion can be had now?

The main issue for me is that the lsim for a DelayLTISystem currently accepts rather different versions of u compared to the old lsim.

DelayLTISystem

  • u is constant or vector (vector is multidim input, not over time)
  • u(uout, t) inplace function
  • u(t)

For a statespace or tf

  • u is matrix of shape (ns, length(t))
  • u(x, t) not inplace, but same signature as delays inplace

I think u in the non-function case should be a matrix of size (ns, length(t)) for both, if you want a constant it is easy to generate a function that returns constant.

For the function I'm not sure, I remember there were some split opinions last time it was brought up about whether lsim should allow for state feedback like this.

@olof3
Copy link
Contributor

olof3 commented Oct 7, 2021

I think u in the non-function case should be a matrix of size (ns, length(t)) for both, if you want a constant it is easy to generate a function that returns constant.

It is not trivial to simulate delay systems with piece-wise constant inputs.

  • Something will probably need to be fixed with how DifferentialEquations is used if we want to use piecewise constant (noncontinuous) inputs, either by using callbacks or doing some "resetting" of the integrator.
  • Use some kind of interpolation. For this option, the user can already do this explicitly.
  • On could use a zero-order hold approximation which would be exact if the delays are multiples of the sample rate, otherwise not. I think it would be a good to have a zero-order hold approximation regardless, but it seems a little bit sketchy that the solution might note be exact (with a warning it could be okay).

For the function I'm not sure, I remember there were some split opinions last time it was brought up about whether lsim should allow for state feedback like this.

This inconsistency is another good reason to get rid of the state feedback possibility, which could instead be invoked in an explicit fashion with a function like sim_feedback. Then one can use the same iip/oop convention as for the delay system case.

DelayLTISystem
* u is constant or vector (vector is multidim input, not over time)

I don't really see how this adds anything, given how easy it is to just use u = t -> u0. But if we want to keep this we should probably allow it for the StateSpace case as well.

We could also think about if we should use the type Returns (in v1.7), it seems like a very convenient as a non-allocating way of providing a constant vector (as long as its return value is not modified, which I don't think we do?!). I guess this would save quite a few allocations for step responses etc.

@albheim
Copy link
Member Author

albheim commented Oct 7, 2021

I think u in the non-function case should be a matrix of size (ns, length(t)) for both, if you want a constant it is easy to generate a function that returns constant.

It is not trivial to simulate delay systems with piece-wise constant inputs.

  • Something will probably need to be fixed with how DifferentialEquations is used if we want to use piecewise constant (noncontinuous) inputs, either by using callbacks or doing some "resetting" of the integrator.
  • Use some kind of interpolation. For this option, the user can already do this explicitly.
  • On could use a zero-order hold approximation which would be exact if the delays are multiples of the sample rate, otherwise not. I think it would be a good to have a zero-order hold approximation regardless, but it seems a little bit sketchy that the solution might note be exact (with a warning it could be okay).

Okay, but then (as you mention below) maybe remove the vector/constant option for the delay systems to not confuse these two cases.

For the function I'm not sure, I remember there were some split opinions last time it was brought up about whether lsim should allow for state feedback like this.

This inconsistency is another good reason to get rid of the state feedback possibility, which could instead be invoked in an explicit fashion with a function like sim_feedback. Then one can use the same iip/oop convention as for the delay system case.

I'm not sure I see why this is a good reason to get rid of it, it would be easy to solve by saying either that we run with u(t) and u(uout, t) as one and two argument options, or we run with u(x, t) and u(uout, x, t) as two and three argument options. As long as we choose either of those cases we won't mix up the methods as easily as now where one could have a u(x, t) funciton, but after adding a delay to the system it is interpreted as a u(uout, t) one.

DelayLTISystem

  • u is constant or vector (vector is multidim input, not over time)

I don't really see how this adds anything, given how easy it is to just use u = t -> u0. But if we want to keep this we should probably allow it for the StateSpace case as well.

Agreed.

We could also think about if we should use the type Returns (in v1.7), it seems like a very convenient as a non-allocating way of providing a constant vector (as long as its return value is not modified, which I don't think we do?!). I guess this would save quite a few allocations for step responses etc.

Don't know what that is, will have a look.

@olof3
Copy link
Contributor

olof3 commented Oct 11, 2021

it would be easy to solve by saying either that we run with u(t) and u(uout, t) as one and two argument options, or we run with u(x, t) and u(uout, x, t) as two and three argument options. As long as we choose either of those cases we won't mix up the methods as easily as now where one could have a u(x, t) funciton, but after adding a delay to the system it is interpreted as a u(uout, t) one.

I'm not sure that I understand your suggestion. Is it that we should decide between either u(t) / u(uout, t) OR u(x, t) / u(uout, x, t) ?

I'm not sure I see why this is a good reason to get rid of it

The main reason is not due to implementation issues, but that I think it is a more clear if u is always treated as a signal by lsim, and then have a different function for simulating state feedback. For example, an LTI system with a transfer function representation the time response is meaningful, but state feedback is not.

@albheim
Copy link
Member Author

albheim commented Oct 12, 2021

I'm not sure that I understand your suggestion. Is it that we should decide between either u(t) / u(uout, t) OR u(x, t) / u(uout, x, t) ?

Yes, that was my thought.

The main reason is not due to implementation issues, but that I think it is a more clear if u is always treated as a signal by lsim, and then have a different function for simulating state feedback. For example, an LTI system with a transfer function representation the time response is meaningful, but state feedback is not.

Okay, that makes sense. So for the above suggestion we go with u(t) / u(uout, t) then.

@olof3
Copy link
Contributor

olof3 commented Oct 13, 2021

So for the above suggestion we go with u(t) / u(uout, t) then.

Yes, I think that is the right way to go. But we would need some sim_statefeedback/sim_statefb function for the state feedback functionality.

@albheim albheim added the v1 Issues to resolve before releasing v1 label Oct 25, 2021
@albheim albheim removed the v1 Issues to resolve before releasing v1 label Mar 14, 2023
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

2 participants