Repository navigation
Erroneous time-domain sim (because of ss2tf) #240
Description
Hi, it seems python-control has a weird bug in the pipeline for converting from MIMO transfer function to state-space form. Consider the below gist
import control as co
import numpy as np
from numpy import array
num = [[array([0.]), array([1.0])],
[array([1.0]), array([0.0])]]
den = [[array([1.]), array([1, 1])],
[array([ 1. , 4.0]), array([1.0])]]
sys = co.tf(num, den)
t, y = co.step_response(sys, input=0)
print("step response (output 1 from input 0) (old): {:}".format(y[1, -1]))
# slightly change the denominator
den = [[array([1.]), array([1e-10, 1, 1])],
[array([ 1. , 4.0]), array([1.0])]]
sys = co.tf(num, den)
t, y = co.step_response(sys, input=0)
print("step response (output 1 from input 0) (changed): {:}".format(y[1, -1]))The output on my computer is
step response (output 1 from input 0) (first): 0.24999999999982692
step response (output 1 from input 0) (second): 0.0
Clearly in the later case, there is a bug in the time-domain simulation function. The DC gain from input 1 to output 1 is 0.25, so the first simulation works alright. But in the second simulation, I obtain zero output. A preliminary investigation suggests that the problem is in the ss2tf procedure, particularly, in the slicot function.
Any suggestions would be great. Current I have to resort to matlab to do the transformation from transfer function to state-space in my application.
Activity
murrayrm commentedon Nov 4, 2018
Confirmed that this is a bug and that the problem is in the conversion to state space form. The following code illustrates the error:
The poles for
sys1tfandsys1ssare -4 and -1. The poles forsys2tfare -1e10, -4, -1 (correct) but forsys2ssthey are -1e10, -1 (incorrect). Not clear how/why the pole at -4 is being dropped. The state space realizations are:So
sys2ssis really wrong...I've created unit tests to catch this error, which can be used once it is fixed. Will commit them as part of the fix.
fix issue python-control#240: _common_den was not padding properly
murrayrm commentedon Nov 4, 2018
Found the problem. Turns out it was not with
slycot(orslicot), but rather with the utility functionxferfcn._common_den(). This function is used to create the input for the functionslycot.td04ad(), which converts a (MIMO) transfer function to a state space system. There are some tricks in the way the coefficients are padded and these had a bug. PR #242 has a fix.hungpham2511 commentedon Nov 5, 2018
Thank you for investigating. Just try a few examples and things seem to be working nicely. I will close this issue.
By the way, may I know if python-control is accepting pr? I need lft (Linear Fractional Transform) and have implemented the function in my fork. Would love to have it reviewed.
murrayrm commentedon Nov 5, 2018
Reopening: this issue is not actually fixed yet. There is a pull request in the works, but it is having unit test errors. Until it gets sorted out, I'll leave this issue open.
murrayrm commentedon Nov 5, 2018
@hungpham2511 Regarding pull requests: you are welcome to add to code base! Please post a PR when you have a chance and someone will have a look.
Merge pull request #242 from murrayrm/fix_common_den
murrayrm commentedon Dec 22, 2018
Fixed in PR #242, now merged.