Skip to content

sisotool small visual cleanup, new feature to show step response of different input-output than loop #531

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 7 commits into from
Feb 7, 2021

Conversation

sawyerbfuller
Copy link
Contributor

@sawyerbfuller sawyerbfuller commented Feb 4, 2021

This PR adds a new feature to sisotool that allows it to show the step response of a system that is not the loop transfer function L. Some visual cleanup including reducing oversized axis tick labels.

  • To see a the step response of a more complicated system that includes a feedforward controller, or any other input-output pair, you can do so by passing sisotool a 2 input, 2-output system. The gain you choose when you click the rlocus plot gets inserted between output[0] and input[0], and then the step response is between input[1] and output[1].
  • Doubled the click threshold size in rlocus because I had a hard time aiming the mouse well enough to get successful click events.

@coveralls
Copy link

coveralls commented Feb 4, 2021

Coverage Status

Coverage increased (+0.3%) to 87.948% when pulling abca69d on sawyerbfuller:sisotooling into 73f65df on python-control:master.

@bnavigator
Copy link
Contributor

👍

@sawyerbfuller
Copy link
Contributor Author

Here's a current screenshot:

image

@bnavigator
Copy link
Contributor

…atibility with matplotlib 1 released 10 years ago
Comment on lines 124 to 137

# test supply tvect
sisotool(sys, tvect=np.arange(0, 1, .1))

# test discrete-time
sisotool(sysdt, tvect=5)

# test MIMO compatibility
# sys must be siso or 2 input, 2 output
with pytest.raises(ControlMIMONotImplemented):
sisotool(sys221)
# does not raise an error:
sisotool(sys222)

Copy link
Contributor

Choose a reason for hiding this comment

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

I would move these to separate tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure, thanks for the suggestions. still getting the hang of best practices for testing

assert_array_almost_equal(
ax_step.lines[0].get_data()[1][:10], step_response_moved, 4)

# test supply tvect
sisotool(sys, tvect=np.arange(0, 1, .1))
Copy link
Contributor

@bnavigator bnavigator Feb 5, 2021

Choose a reason for hiding this comment

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

This still does not cover https://coveralls.io/builds/36911686/source?filename=control%2Fsisotool.py#L168

tvect is not passed through to _SisotoolUpdate

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's see if that works

@bnavigator bnavigator merged commit 3adc610 into python-control:master Feb 7, 2021
@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