GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Add slicing access for state-space models with tests by guptavaibhav0 · Pull Request #1012 · python-control/python-control

Conversation

@guptavaibhav0

@guptavaibhav0 guptavaibhav0 commented Jun 17, 2024

Copy link
Copy Markdown
Contributor

This PR adds the capability to access state-space models using splice. This should help fix #256.

Current Issue:

>>> b =  ct.ss([[2, 3], [3, 4]], [[2, 1], [2, 3]], [[2, 4]], [[1, 2]], 0.1)
>>> b[:2, 1]
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File ".../.venv/lib/python3.12/site-packages/control/statesp.py", line 1224, in __getitem__
    self.A, self.B[:, inpdx], self.C[outdx, :], self.D[outdx, inpdx],
            ~~~~~~^^^^^^^^^^
IndexError: only integers, slices (`:`), ellipsis (`...`), numpy.newaxis (`None`) and integer or boolean arrays are valid indices

Changes in this PR:

  • Adds a small check for indexing of transfer functions.
  • Adds slicing in the state-space model with proper tests.
>>> b =  ct.ss([[2, 3], [3, 4]], [[2, 1], [2, 3]], [[2, 4]], [[1, 2]], 0.1)
>>> print(b[:2, 1])
<StateSpace>: sys[0]$indexed
Inputs (1): ['u[1]']
Outputs (1): ['y[0]']
States (2): ['x[0]', 'x[1]']

A = [[2. 3.]
     [3. 4.]]

B = [[1.]
     [3.]]

C = [[2. 4.]]

D = [[2.]]

dt = 0.1

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@coveralls

coveralls commented Jun 18, 2024

Copy link
Copy Markdown

Coverage Status

coverage: 94.473% (-0.02%) from 94.49%
when pulling 0f08b1e on guptavaibhav0:main
into dbc998d on python-control:main.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@murrayrm

murrayrm commented Jun 26, 2024

Copy link
Copy Markdown
Member

@ guptavaibhav0 Please rebase this on the last main branch to allow the CI tests to run. Please note that the failure of "Conda-based pytest / Py3.10; conda Slycot; no Pandas; no CVXOPT" looks like something unrelated and should clear up when the tests are re-run.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@guptavaibhav0
Add slicing access for state-space models with tests
6b61ed0
@guptavaibhav0
guptavaibhav0 force-pushed the main branch from 0f03778 to 6b61ed0 Compare June 26, 2024 16:49
@guptavaibhav0

guptavaibhav0 commented Jun 26, 2024

Copy link
Copy Markdown
Contributor Author

@murrayrm I have done the rebase and hopefully, this should fix it!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@coveralls

coveralls commented Jun 26, 2024

Copy link
Copy Markdown

Coverage Status

coverage: 94.505% (-0.02%) from 94.522%
when pulling 6b61ed0 on guptavaibhav0:main
into ad6b49e on python-control:main.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
murrayrm
murrayrm reviewed Jun 26, 2024
View reviewed changes
Comment thread control/xferfcn.py Outdated

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

There was a problem hiding this comment.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@guptavaibhav0 guptavaibhav0 Jun 27, 2024

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread control/tests/statesp_test.py
Comment on lines +477 to +480
[(0, 1),
(slice(0, 1, 1), 1),
(0, slice(1, 2, 1)),
(slice(0, 1, 1), slice(1, 2, 1))])

@murrayrm murrayrm Jun 26, 2024

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

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]?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@guptavaibhav0 guptavaibhav0 Jun 27, 2024

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread control/statesp.py Outdated
Comment on lines +1226 to +1228
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

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@guptavaibhav0 guptavaibhav0 Jun 27, 2024

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
guptavaibhav0 added 3 commits June 27, 2024 12:15
@guptavaibhav0
Correct typos and column length issue
4dff649
@guptavaibhav0
Fixed bug in statespace initilisation
c6ef9b4
@guptavaibhav0
Added more test cases for slicing of statespace model
a0fc6bc
@coveralls

coveralls commented Jun 27, 2024

Copy link
Copy Markdown

Coverage Status

coverage: 94.506% (-0.02%) from 94.522%
when pulling a0fc6bc on guptavaibhav0:main
into ad6b49e on python-control:main.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@murrayrm
murrayrm merged commit e5394c4 into python-control:main Jun 27, 2024
@murrayrm murrayrm added this to the 0.10.1 milestone Jun 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@murrayrm murrayrm murrayrm left review comments

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

0.10.1

Development

Successfully merging this pull request may close these issues.

Create functions for better indexing of MIMO transfer functions

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@guptavaibhav0 @coveralls @murrayrm
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.