Repository navigation
A bug in bdalg.connect function #421
Description
Activity
If you can generate a simple example that illustrates the bug, this would help us create a unit test to go along with the fix.
It looks to me that the intention of the current code is to neglect interconnections out of range .
Hence, the result will be the same as that only with valid interconnections.I made test code like this:
import numpy as np from control.statesp import StateSpace from control.bdalg import append, connect class Test(object): def __init__(self): self.sys1 = StateSpace([[1., -2], [3., -4]], [[5.], [7]], [[6, 8]], [[9.]]) self.sys2 = StateSpace([[-1.]], [[1.]], [[1.]], [[0.]]) def test_wrong_Q_arg(self): sys = append(self.sys1, self.sys2) # wrong negative feedback interconnection will be neglected Q1 = [[1, 2], [2, -3]] ans1 = connect(sys, Q1, [2], [1, 2]) Q2 = [[1, 2]] ans2 = connect(sys, Q2, [2], [1, 2]) np.testing.assert_array_almost_equal(ans1.A, ans2.A) np.testing.assert_array_almost_equal(ans1.B, ans2.B) np.testing.assert_array_almost_equal(ans1.C, ans2.C) np.testing.assert_array_almost_equal(ans1.D, ans2.D) Test().test_wrong_Q_arg()
However, the test ends with IndexError.
Volumes/ssd1/Applications/anaconda3/lib/python3.7/site-packages/control/bdalg.py in connect(sys, Q, inputv, outputv) 334 K[inp,outp-1] = 1. 335 elif outp < 0 and -outp >= -sys.outputs: --> 336 K[inp,-outp-1] = -1. 337 sys = sys.feedback(np.array(K), sign=1) 338 IndexError: index 2 is out of bounds for axis 1 with size 2I guess the code needs a range check of the variable
inpalso.I encountered a similar issue, but I'm not sure if it's the same:
plant = control.ss(control.tf(1, [2, 1])) compensator = control.tf([10, 10], [100, 1]) filter = control.tf(1, [10, 1]) summer = control.ss([[0,0.],[0,0]], [[0,0.],[0,0]], [[0,0.],[0,0]], [[1, 0],[1, 1]]) FFGain = control.tf(Kff,1) appended = control.append(plant, summer, FFGain, compensator, filter) connects = [ [1, +6, 0], [4, -1, 0], [5, +2, 0], [6, +3, 0], [7, +4, +5]] inpts = [ 3, 2 ] # [1] r input, [2] d (disturbance input) outpts = [ 1, ] # [1] y (plant output) # bug/problem here: closedloop = control.connect(appended, connects, inpts, outpts)gives
~/Documents/src/python-control-dev/python-control/control/bdalg.py in connect(sys, Q, inputv, outputv) 332 for outp in r[1:]: 333 if outp > 0 and outp <= sys.outputs: --> 334 K[inp,outp-1] = 1. 335 elif outp < 0 and -outp >= -sys.outputs: 336 K[inp,-outp-1] = -1. IndexError: index 6 is out of bounds for axis 0 with size 6Do we want to ignore out-of-range input and output indices here, or raise an error? I'd lean towards reporting an error. This applies to Q, inputv, and outputv.
Agree, it should raise an error. I'm bogged down in a bunch of other things right now, but hope to have some time first week of September. If someone else wants to take a look between now and then, that would be great.
I second Richard’s sentiment - should raise an error.
Ok I think I fixed it in #474. The error that I found above that I thought was related was actually in other code.
Dear all,
I found a bug in the connect function in the bdalg.py:
335: elif outp < 0 and -outp >= -sys.outputs:
if outp < 0, the index meant is -outp because the negative sign is to tell it is negative feedback.
(-outp) must be in the range [1, sys.outputs]. As outp < 0, -outp > 0 already. Hence, the remaining thing is that -outp <= sys.outputs. The code must be corrected like this:
335: elif outp < 0 and -outp <= sys.outputs:
Regards.
Seung-Kil