Skip to content

Add slicing access for state-space models with tests #1012

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
murrayrm merged 4 commits into from
Jun 27, 2024
Merged

Add slicing access for state-space models with tests #1012

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6b61ed0
Add slicing access for state-space models with tests
guptavaibhav0 Jun 17, 2024
4dff649
Correct typos and column length issue
guptavaibhav0 Jun 27, 2024
c6ef9b4
Fixed bug in statespace initilisation
guptavaibhav0 Jun 27, 2024
a0fc6bc
Added more test cases for slicing of statespace model
guptavaibhav0 Jun 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions control/statesp.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
import math
from copy import deepcopy
from warnings import warn
from collections.abc import Iterable

import numpy as np
import scipy as sp
Expand Down Expand Up @@ -1215,17 +1216,16 @@ def append(self, other):

def __getitem__(self, indices):
"""Array style access"""
if len(indices) != 2:
if not isinstance(indices, Iterable) or len(indices) != 2:
raise IOError('must provide indices of length 2 for state space')
outdx = indices[0] if isinstance(indices[0], list) else [indices[0]]
inpdx = indices[1] if isinstance(indices[1], list) else [indices[1]]
outdx, inpdx = indices
if not isinstance(outdx, (int, slice)) or not isinstance(inpdx, (int, slice)):
raise TypeError(f"system indices must be integers or slices")
sysname = config.defaults['iosys.indexed_system_name_prefix'] + \
self.name + config.defaults['iosys.indexed_system_name_suffix']
return StateSpace(
self.A, self.B[:, inpdx], self.C[outdx, :], self.D[outdx, inpdx],
self.dt, name=sysname,
inputs=[self.input_labels[i] for i in list(inpdx)],
outputs=[self.output_labels[i] for i in list(outdx)])
self.dt, name=sysname, inputs=self.input_labels[inpdx], outputs=self.output_labels[outdx])

@murrayrm murrayrm Jun 26, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was there a reason for this change? It looks like the updated final line goes past column 79, which we try not to do.

@guptavaibhav0 guptavaibhav0 Jun 27, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change was needed since slice is not iterable. I have pushed a new commit with a maximum column length of 79.


def sample(self, Ts, method='zoh', alpha=None, prewarp_frequency=None,
name=None, copy_names=True, **kwargs):
Expand Down
22 changes: 18 additions & 4 deletions control/tests/statesp_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -463,24 +463,38 @@ def test_append_tf(self):
np.testing.assert_array_almost_equal(sys3c.A[:3, 3:], np.zeros((3, 2)))
np.testing.assert_array_almost_equal(sys3c.A[3:, :3], np.zeros((2, 3)))

def test_array_access_ss(self):

def test_array_access_ss_failure(self):
sys1 = StateSpace(
[[1., 2.], [3., 4.]],
[[5., 6.], [6., 8.]],
[[9., 10.], [11., 12.]],
[[13., 14.], [15., 16.]], 1,
inputs=['u0', 'u1'], outputs=['y0', 'y1'])
with pytest.raises(IOError):
sys1[0]

@pytest.mark.parametrize("outdx, inpdx",
[(0, 1),
(slice(0, 1, 1), 1),
(0, slice(1, 2, 1)),
(slice(0, 1, 1), slice(1, 2, 1))])
Comment on lines +477 to +486

@murrayrm murrayrm Jun 26, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to add some more general tests. For example, how about [:2, 1] or [::-1, ::2]?

@guptavaibhav0 guptavaibhav0 Jun 27, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added some more general tests in the new commit. I also found a bug in the StateSpace init function which I have corrected. The init function doesn't check if the dimensions of the D matrix are correct if the input/output names are given.

def test_array_access_ss(self, outdx, inpdx):
sys1 = StateSpace(
[[1., 2.], [3., 4.]],
[[5., 6.], [6., 8.]],
[[9., 10.], [11., 12.]],
[[13., 14.], [15., 16.]], 1,
inputs=['u0', 'u1'], outputs=['y0', 'y1'])

sys1_01 = sys1[0, 1]
sys1_01 = sys1[outdx, inpdx]
np.testing.assert_array_almost_equal(sys1_01.A,
sys1.A)
np.testing.assert_array_almost_equal(sys1_01.B,
sys1.B[:, 1:2])
np.testing.assert_array_almost_equal(sys1_01.C,
sys1.C[0:1, :])
np.testing.assert_array_almost_equal(sys1_01.D,
sys1.D[0, 1])
sys1.D[0:1, 1:2])

assert sys1.dt == sys1_01.dt
assert sys1_01.input_labels == ['u1']
Expand Down
7 changes: 7 additions & 0 deletions control/xferfcn.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@

"""

from collections.abc import Iterable

# External function declarations
import numpy as np
from numpy import angle, array, empty, finfo, ndarray, ones, \
Expand Down Expand Up @@ -758,7 +760,12 @@ def __pow__(self, other):
return (TransferFunction([1], [1]) / self) * (self**(other + 1))

def __getitem__(self, key):
if not isinstance(key, Iterable) or len(key) != 2:
raise IOError('must provide indices of length 2 for state space')

@murrayrm murrayrm Jun 26, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change "state space" to "transfer functions".

@guptavaibhav0 guptavaibhav0 Jun 27, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have updated in the latest commit.


key1, key2 = key
if not isinstance(key1, (int, slice)) or not isinstance(key2, (int, slice)):
raise TypeError(f"system indices must be integers or slices")

# pre-process
if isinstance(key1, int):
Expand Down