Skip to content

change default value of statesp.remove_useless_states to False #509

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

Merged
merged 2 commits into from
Jan 15, 2021

Conversation

murrayrm
Copy link
Member

@murrayrm murrayrm commented Jan 15, 2021

This PR resolves issue #244. It looks like the only issue was that converting a constant to a state space object was creating an extra state and this state had to then be removed. By creating a state space system with no internal state (A = [[]] instead of A=[[0]]), this action is no longer required to get sensible results from block diagram algebra functions.

The PR makes that change and also changes the default value of statesp.remove_useless_states to False.

@coveralls
Copy link

coveralls commented Jan 15, 2021

Coverage Status

Coverage increased (+0.1%) to 87.65% when pulling b7945f3 on murrayrm:dont_remove_useless_states into ad0a0eb on python-control:master.

@@ -395,9 +395,6 @@ def test_is_static_gain(self):
D0 = 0
D1 = np.ones((2,1))
assert StateSpace(A0, B0, C1, D1).is_static_gain()
# TODO: fix this once remove_useless_states is false by default
# should be False when remove_useless is false
# print(StateSpace(A1, B0, C1, D1).is_static_gain())
Copy link
Contributor

Choose a reason for hiding this comment

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

assert not StateSpace(A1, B0, C1, D1).is_static_gain() ?

@bnavigator bnavigator merged commit b47ed08 into python-control:master Jan 15, 2021
@murrayrm murrayrm deleted the dont_remove_useless_states branch January 15, 2021 22:03
@murrayrm murrayrm added this to the 0.9.0 milestone Mar 20, 2021
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.

4 participants