fix issue #240: _common_den was not padding properly#242
fix issue #240: _common_den was not padding properly#242murrayrm merged 1 commit intopython-control:masterfrom
Conversation
1 similar comment
|
Unit tests are failing in LQR checks. The test case is based on a state space system that is computed from a transfer function, so could be a problem in this PR (but it worked on my local machine, so something odd is going on). Will look into the issue and try to fix later. |
|
Not able to replicate the unit test failure under MacOS. Tried python3.5 and python3.7, but all unit tests pass. Will have to start up a linux instance and see if I can generate the error. |
|
Tried recreating the error on Ubuntu linux (trusty), but all unit tests seemed to work. ?? Also, |
|
I checked out the PR and tested locally; looks good and correct results (Fedora 27). I also can't figure out why the tests fail on travis. |
This PR fixes a problem where
xferfcn._common_denwas not properly padding the numerator coefficients and this was causing an apparent bug intf2ssfor MIMO systems (see issue #240). The issue occurred in the following code inxferfcn.py:where
npmaxis the maximum number of poles across all inputs.What this code is doing is putting the coefficients for
numpolyat the right end of the arraynum[i,j], which has lengthnpmax+1. However, whattd04adlooks at is only the portion of the numerator array that is the order of the denominator array for that particular input (not the max across all inputs). So this code was shifting things too far to the right in the case where you had an input that had less than the maximum number of poles (discovered in issue #240, as part of a simulation bug).The new code is
where
npis the number of poles for this input (and hencenp+1is the portion of the input array thattd04adwill use.In addition to fixing this bug, two unit tests were added:
control/tests/convert_test.pyhas a test to make sure that_common_dendoes the right thing.control/tests/timeresp_test.pyhas a test that corresponds to the original bug identified in issue Erroneous time-domain sim (because of ss2tf) #240.@repagh should have a look at this since it is in a section of code that he wrote.