A bug in bdalg.connect function

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

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 2

I guess the code needs a range check of the variable inp also.

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 6

Do 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.

bdalg.connect: added and fixed index checks to fix python-control#421;…

Ok I think I fixed it in #474. The error that I found above that I thought was related was actually in other code.

Navigation Menu

Repository navigation

A bug in bdalg.connect function #421

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

A bug in bdalg.connect function #421

Description

Activity

murrayrm commented on Jun 20, 2020

SeungKil-Paek commented on Jun 21, 2020

sawyerbfuller commented on Jun 22, 2020

roryyorke commented on Aug 22, 2020

murrayrm commented on Aug 22, 2020

sawyerbfuller commented on Aug 22, 2020

sawyerbfuller commented on Dec 19, 2020

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions