Description

sergio-dorado

opened on Jun 12, 2020

Issue body actions

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.

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.

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

Kind regards,

Sergio Dorado-Rojas

Activity

1. murrayrm commented on Jun 13, 2020

   

   murrayrm

   on Jun 13, 2020

   Member

   More actions

   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).
2. murrayrm

   added this to the 0.8.4 milestone on Jul 11, 2020
3. murrayrm

   added

   enhancement

   on Jul 11, 2020
4. murrayrm

   self-assigned this

   on Jul 11, 2020
5. sawyerbfuller

   linked a pull request that will close this issueDefault dt is 0 #431on Aug 19, 2020
6. murrayrm

   modified the milestones: 0.8.4, 0.9.0 on Oct 17, 2020
7. bnavigator

   linked a pull request that will close this issueRebased #431: Default dt is 0 #490on Dec 31, 2020
8. murrayrm commented on Dec 31, 2020

   

   murrayrm

   on Dec 31, 2020

   Member

   More actions

   Confirmed that this is fixed by #431.
9. murrayrm

   added

   PR pending

   on Dec 31, 2020
10. bnavigator

    closed this as completedin #490on Dec 31, 2020

Issue actions