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

Problem with DT State-Space Response #419

Closed
sergio-dorado opened this issue Jun 12, 2020 · 2 comments · Fixed by #490
Closed

Problem with DT State-Space Response #419

sergio-dorado opened this issue Jun 12, 2020 · 2 comments · Fixed by #490
Assignees
Milestone

Comments

@sergio-dorado
Copy link

Hi all,

First, congratulations for the wonderful work you've done by maintaining and creating the library!

I have found some issues working with DTLTI systems. More specifically, I am trying to simulate the step response of a DT system directly from its state-space representation. This realization is computed directly in the DT domain, without a sampling process. As you can see in the image below, there is a problem since the step response is blowing up. My intuition is that there is a bug since the DT system is being treated as if it were CT, but it is DT.

Screen Shot 2020-06-12 at 3 38 30 PM

Exactly the same happens when the dt parameter is set to None. When I try to evaluate whether the SS representation is DT or CT, I get that it's being taken as both.

image

I would appreciate any guidelines on how to fix this. I can do it myself and pull it :)

Kind regards,

Sergio Dorado-Rojas

@murrayrm
Copy link
Member

The problem is that H_ss_direct is not actually a discrete time system, even though they way you created it makes it look like it should be. The actually calling signature for control.StateSpace is

StateSpace(A, B, C, D[, dt])

The final dt argument is not a keyword argument but a positional argument. Hence the say to create H_ss_direct is

H_ss_direct = ctrl.StateSpace(A, B, C, D, True) 

In the call that you made, H_ss_direct has timebase None which means that it can represent either a continuous time system or a discrete time system. The various time response functions all assume that if you don't specify a timebase then it is continuous. (If you want this type of "strict" checking in isdtime, you can use strict=True when you call isdtime.)

At a minimum, there should be an error message printed with the call that you made, saying that dt was not recognized. Better (I think) would be to recognize the dt keyword since it is a completely natural (and perhaps even better) way to create a discrete time system.

It would be great if you want to submit a PR to fix this (remember to include docstrings and some unit tests that both capture the only case and test all new code that you add).

@murrayrm murrayrm added this to the 0.8.4 milestone Jul 11, 2020
@murrayrm murrayrm self-assigned this Jul 11, 2020
@sawyerbfuller sawyerbfuller linked a pull request Aug 19, 2020 that will close this issue
@murrayrm murrayrm modified the milestones: 0.8.4, 0.9.0 Oct 17, 2020
@bnavigator bnavigator linked a pull request Dec 31, 2020 that will close this issue
@murrayrm
Copy link
Member

Confirmed that this is fixed by #431.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants