Skip to content

added xperm function: reorder state variables in a ss model. #1039

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.

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

toaster-code
Copy link

This PR provide a new function xperm, which allows to reorder state variables in a ss model. It is still undocumented (WIP) and there are caveats (named arguments does not propagate to the new ss model created as output from this function).
This PR contains no changes in functionality of the rest of the code, just adds a function that exists in Matlab control system toolbox.

Use case: Obtain the exact set of matrices A,B,C,D from a ss model without recurring to manually reorder them to match a result (like a book exercise).

There are other users cases I think, but for me, I wanted to have xperm as MATLAB does to reorder my obtained matrices to match the book results exactly.

Copy link
Member

@slivingston slivingston left a comment

Choose a reason for hiding this comment

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

Please add tests.

D_perm = sys.D # D remains unchanged

return ss(A_perm, B_perm, C_perm, D_perm)
"""
Copy link
Member

Choose a reason for hiding this comment

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

This string block should be moved to somewhere accessible to users: the docstring, or a file in examples/.


"""
# TODO: transfer the original sys parameters to the new output sys to preserve labels.
# TODO: create docstrings for this function - WIP
Copy link
Member

Choose a reason for hiding this comment

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

This will not be merged without a substantial docstring.

@coveralls
Copy link

Coverage Status

coverage: 94.421% (-0.3%) from 94.694%
when pulling c102929 on toaster-code:added_xperm_function
into 373ff11 on python-control:main.

@toaster-code
Copy link
Author

@slivingston Thanks for the feedback.
Sure. I shall try to write some examples to the folder "examples" and write some docstrings.
This is my first time contributing to a substantial project, so sorry if the contribution is still raw or incomplete.
All tips and advises are welcome.

I shall improve it locally before re-submitting a Pull request.

Thanks.

@slivingston slivingston self-assigned this Aug 13, 2024
@slivingston
Copy link
Member

Sure. I shall try to write some examples to the folder "examples" and write some docstrings. This is my first time contributing to a substantial project, so sorry if the contribution is still raw or incomplete. All tips and advises are welcome.

I shall improve it locally before re-submitting a Pull request.

Thanks for contributing! No worries, and I am happy to provide feedback. We can iterate on this PR as much as needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants