Skip to content

Unit tests for gangof4_plot #505

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

Closed
wants to merge 8 commits into from
Closed

Unit tests for gangof4_plot #505

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
d0f29fc
Create python-package.yml
murrayrm Jan 10, 2021
2d44387
Merge branch 'master' of https://github.com/murrayrm/python-control
murrayrm Jan 10, 2021
97c71d6
Merge branch 'master' of https://github.com/python-control/python-con…
murrayrm Jan 13, 2021
a64f7f1
Revert "Create python-package.yml"
murrayrm Jan 13, 2021
362f46d
add gangof4 unit test using mpl.image_comparison
murrayrm Jan 9, 2021
518c061
add pytest.ini file with customer marker to avoid mpl warning
murrayrm Jan 9, 2021
2d1650d
import nose to avoid Python 2.7/matplotlib.testing error
murrayrm Jan 10, 2021
b300886
address bnavigator review comments: image symlink + pytest.ini -> set…
murrayrm Jan 13, 2021
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
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,4 @@ Untitled*.ipynb
# Files created by or for emacs (RMM, 29 Dec 2017)
*~
TAGS
result_images/
4 changes: 4 additions & 0 deletions .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,10 @@ before_install:
# Make sure to look in the right place for python libraries (for slycot)
- export LIBRARY_PATH="$HOME/miniconda/envs/test-environment/lib"
- conda install pytest
# matplotlib.testing.image_compare requires nose in Python 2.7
- if [[ "$TRAVIS_PYTHON_VERSION" == "2.7" ]]; then
conda install nose;
fi
# coveralls not in conda repos => install via pip instead
- pip install coveralls

Expand Down
1 change: 1 addition & 0 deletions control/tests/baseline_images/freqresp_test/gangof4-pvtol-expected.png
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
gangof4-pvtol.png
Binary file added control/tests/baseline_images/freqresp_test/gangof4-pvtol.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
27 changes: 26 additions & 1 deletion control/tests/freqresp_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@
from control.statesp import StateSpace
from control.xferfcn import TransferFunction
from control.matlab import ss, tf, bode, rss
from control.tests.conftest import slycotonly
from control.tests.conftest import slycotonly, nopython2

from matplotlib.testing.decorators import image_comparison

pytestmark = pytest.mark.usefixtures("mplcleanup")

Expand Down Expand Up @@ -345,3 +347,26 @@ def test_phase_wrap(TF, wrap_phase, min_phase, max_phase):
mag, phase, omega = ctrl.bode(TF, wrap_phase=wrap_phase)
assert(min(phase) >= min_phase)
assert(max(phase) <= max_phase)

@pytest.fixture
def pvtol_inner():
m = 4 # mass of aircraft
J = 0.0475 # inertia around pitch axis
r = 0.25 # distance to center of force
g = 9.8 # gravitational constant
c = 0.05 # damping factor (estimated)

# Transfer functions for dynamics
Pi = ctrl.tf([r], [J, 0, 0]) # inner loop (roll)

k, a, b = 200, 2, 50 # control parameters
Ci = k * ctrl.tf([1, a], [1, b]) # lead compensator

return (Pi, Ci)

# Regression test for Gang of 4 plots
@nopython2
Copy link
Contributor

Choose a reason for hiding this comment

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

Why install nose in python2 then? Just for the import?

Copy link
Member Author

@murrayrm murrayrm Jan 13, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Exactly: the problem I was having is that I need to import image_comparison from matplotlib.testing.decorators and this requires nose under python2 (no idea why). And I think there was some problem with getting the comparison to work in Python2 (at some point), so I skipped the unit test.

We can probably clean it up, but since it might break Travis and since we are deprecating Python 2, I'm going to leave for now.

Copy link
Contributor

Choose a reason for hiding this comment

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

I hope we can get rid of Python 2 soon.

Copy link
Contributor

@henklaak henklaak Feb 2, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for the effort, but... in my experience image comparisons with matplotlib outputs are very brittle.
Whenever matplotlib decides to change their defaults (which they have done in the past), or a font is ever so slightly different, the test will fail. It's a never ending maintenance hassle.

Probably better to mock the matplotlib interface and only check for the right interface calls to mpl.
I'm willing to help.

Copy link
Contributor

@henklaak henklaak Feb 2, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

To help you on your way:

from unittest.mock import patch

def test_gangof4_pvtol(pvtol_inner):
    with patch('control.freqplot.plt') as mockplt:
        ctrl.gangof4(*pvtol_inner)
        print(mockplt.mock_calls)

The output of this looks something like this:

call.gcf().axes.__iter__(),
 call.clf(),
 call.subplot(221, label='control-gangof4-s'),
 call.subplot(222, label='control-gangof4-ps'),
 call.subplot(223, label='control-gangof4-cs'),
 call.subplot(224, label='control-gangof4-t'),
 call.subplot().loglog(array([1.00000000e-01, 1.20679264e-01, 1.45634848e-01, 1.75751062e-01,
       2.12095089e-01, 2.55954792e-01, 3.08884360e-01, 3.72759372e-01,
       4.49843267e-01, 5.42867544e-01, 6.55128557e-01, 7.90604321e-01,
       9.54095476e-01, 1.15139540e+00, 1.38949549e+00, 1.67683294e+00,
       2.02358965e+00, 2.44205309e+00, 2.94705170e+00, 3.55648031e+00,
       4.29193426e+00, 5.17947468e+00, 6.25055193e+00, 7.54312006e+00,
       9.10298178e+00, 1.09854114e+01, 1.32571137e+01, 1.59985872e+01,
       1.93069773e+01, 2.32995181e+01, 2.81176870e+01, 3.39322177e+01,
       4.09491506e+01, 4.94171336e+01, 5.96362332e+01, 7.19685673e+01,
       8.68511374e+01, 1.04811313e+02, 1.26485522e+02, 1.52641797e+02,
       1.84206997e+02, 2.22299648e+02, 2.68269580e+02, 3.23745754e+02,
       3.90693994e+02, 4.71486636e+02, 5.68986603e+02, 6.86648845e+02,
       8.28642773e+02, 1.00000000e+03]), array([2.37260369e-04, 3.45374869e-04, 5.02649698e-04, 7.31321247e-04,
       1.06355413e-03, 1.54573109e-03, 2.24444103e-03, 3.25466960e-03,
       4.71066442e-03, 6.79967979e-03, 9.77807906e-03, 1.39878108e-02,
       1.98689733e-02, 2.79616651e-02, 3.88897504e-02, 5.33242435e-02,
       7.19369207e-02, 9.53699704e-02, 1.24250287e-01, 1.59257279e-01,
       2.01222831e-01, 2.51226612e-01, 3.10656635e-01, 3.81214259e-01,
       4.64827654e-01, 5.63376984e-01, 6.78010385e-01, 8.07663375e-01,
       9.46435949e-01, 1.08059131e+00, 1.18899261e+00, 1.25212430e+00,
       1.26542868e+00, 1.24175520e+00, 1.20028019e+00, 1.15590007e+00,
       1.11647717e+00, 1.08468611e+00, 1.06045859e+00, 1.04263298e+00,
       1.02981186e+00, 1.02072737e+00, 1.01435479e+00, 1.00991477e+00,
       1.00683547e+00, 1.00470657e+00, 1.00323791e+00, 1.00222621e+00,
       1.00152999e+00, 1.00105121e+00])),
 call.subplot().set_ylabel(''),
 call.subplot().tick_params(labelbottom=False),
 call.subplot().grid(True, which='both'),
 call.subplot().loglog(array([1.00000000e-01, 1.20679264e-01, 1.45634848e-01, 1.75751062e-01,
       2.12095089e-01, 2.55954792e-01, 3.08884360e-01, 3.72759372e-01,
       4.49843267e-01, 5.42867544e-01, 6.55128557e-01, 7.90604321e-01,
       9.54095476e-01, 1.15139540e+00, 1.38949549e+00, 1.67683294e+00,
       2.02358965e+00, 2.44205309e+00, 2.94705170e+00, 3.55648031e+00,
       4.29193426e+00, 5.17947468e+00, 6.25055193e+00, 7.54312006e+00,
       9.10298178e+00, 1.09854114e+01, 1.32571137e+01, 1.59985872e+01,
       1.93069773e+01, 2.32995181e+01, 2.81176870e+01, 3.39322177e+01,
       4.09491506e+01, 4.94171336e+01, 5.96362332e+01, 7.19685673e+01,
       8.68511374e+01, 1.04811313e+02, 1.26485522e+02, 1.52641797e+02,
       1.84206997e+02, 2.22299648e+02, 2.68269580e+02, 3.23745754e+02,
       3.90693994e+02, 4.71486636e+02, 5.68986603e+02, 6.86648845e+02,
       8.28642773e+02, 1.00000000e+03]), array([1.24873879e-01, 1.24816450e-01, 1.24732955e-01, 1.24611657e-01,
       1.24435636e-01, 1.24180616e-01, 1.23812007e-01, 1.23281004e-01,
       1.22519754e-01, 1.21435903e-01, 1.19907647e-01, 1.17781690e-01,
       1.14878340e-01, 1.11009578e-01, 1.06014988e-01, 9.98140114e-02,
       9.24598842e-02, 8.41681618e-02, 7.52953730e-02, 6.62681540e-02,
       5.74933390e-02, 4.92879337e-02, 4.18495024e-02, 3.52625322e-02,
       2.95237153e-02, 2.45704351e-02, 2.03041487e-02, 1.66078542e-02,
       1.33631565e-02, 1.04764464e-02, 7.91528160e-03, 5.72360105e-03,
       3.97186315e-03, 2.67625372e-03, 1.77626855e-03, 1.17457560e-03,
       7.79013934e-04, 5.19677785e-04, 3.48865868e-04, 2.35521695e-04,
       1.59732041e-04, 1.08712281e-04, 7.41811267e-05, 5.07134269e-05,
       3.47161403e-05, 2.37873940e-05, 1.63097105e-05, 1.11877503e-05,
       7.67671937e-06, 5.26869060e-06])),
 call.subplot().tick_params(labelbottom=False),
 call.subplot().set_ylabel(''),
 call.subplot().grid(True, which='both'),
 call.subplot().loglog(array([1.00000000e-01, 1.20679264e-01, 1.45634848e-01, 1.75751062e-01,
       2.12095089e-01, 2.55954792e-01, 3.08884360e-01, 3.72759372e-01,
       4.49843267e-01, 5.42867544e-01, 6.55128557e-01, 7.90604321e-01,
       9.54095476e-01, 1.15139540e+00, 1.38949549e+00, 1.67683294e+00,
       2.02358965e+00, 2.44205309e+00, 2.94705170e+00, 3.55648031e+00,
       4.29193426e+00, 5.17947468e+00, 6.25055193e+00, 7.54312006e+00,
       9.10298178e+00, 1.09854114e+01, 1.32571137e+01, 1.59985872e+01,
       1.93069773e+01, 2.32995181e+01, 2.81176870e+01, 3.39322177e+01,
       4.09491506e+01, 4.94171336e+01, 5.96362332e+01, 7.19685673e+01,
       8.68511374e+01, 1.04811313e+02, 1.26485522e+02, 1.52641797e+02,
       1.84206997e+02, 2.22299648e+02, 2.68269580e+02, 3.23745754e+02,
       3.90693994e+02, 4.71486636e+02, 5.68986603e+02, 6.86648845e+02,
       8.28642773e+02, 1.00000000e+03]), array([1.90045028e-03, 2.76801618e-03, 4.03182734e-03, 5.87307962e-03,
       8.55606554e-03, 1.24665394e-02, 1.81680607e-02, 2.64849951e-02,
       3.86252369e-02, 5.63624073e-02, 8.23073431e-02, 1.20313377e-01,
       1.76080109e-01, 2.58045753e-01, 3.78687011e-01, 5.56378770e-01,
       8.18017500e-01, 1.20271551e+00, 1.76706084e+00, 2.59269630e+00,
       3.79723689e+00, 5.54974746e+00, 8.09203498e+00, 1.17664714e+01,
       1.70487195e+01, 2.45764570e+01, 3.51462684e+01, 4.96104601e+01,
       6.85493958e+01, 9.16201542e+01, 1.16854860e+02, 1.40868317e+02,
       1.60548552e+02, 1.74721045e+02, 1.84058879e+02, 1.89930635e+02,
       1.93569151e+02, 1.95834494e+02, 1.97264771e+02, 1.98183002e+02,
       1.98781882e+02, 1.99177719e+02, 1.99442118e+02, 1.99620129e+02,
       1.99740677e+02, 1.99822653e+02, 1.99878563e+02, 1.99916776e+02,
       1.99942930e+02, 1.99960848e+02])),
 call.subplot().set_xlabel('Frequency (rad/sec)'),
 call.subplot().set_ylabel(''),
 call.subplot().grid(True, which='both'),
 call.subplot().loglog(array([1.00000000e-01, 1.20679264e-01, 1.45634848e-01, 1.75751062e-01,
       2.12095089e-01, 2.55954792e-01, 3.08884360e-01, 3.72759372e-01,
       4.49843267e-01, 5.42867544e-01, 6.55128557e-01, 7.90604321e-01,
       9.54095476e-01, 1.15139540e+00, 1.38949549e+00, 1.67683294e+00,
       2.02358965e+00, 2.44205309e+00, 2.94705170e+00, 3.55648031e+00,
       4.29193426e+00, 5.17947468e+00, 6.25055193e+00, 7.54312006e+00,
       9.10298178e+00, 1.09854114e+01, 1.32571137e+01, 1.59985872e+01,
       1.93069773e+01, 2.32995181e+01, 2.81176870e+01, 3.39322177e+01,
       4.09491506e+01, 4.94171336e+01, 5.96362332e+01, 7.19685673e+01,
       8.68511374e+01, 1.04811313e+02, 1.26485522e+02, 1.52641797e+02,
       1.84206997e+02, 2.22299648e+02, 2.68269580e+02, 3.23745754e+02,
       3.90693994e+02, 4.71486636e+02, 5.68986603e+02, 6.86648845e+02,
       8.28642773e+02, 1.00000000e+03]), array([1.00023699e+00, 1.00034480e+00, 1.00050143e+00, 1.00072873e+00,
       1.00105808e+00, 1.00153419e+00, 1.00222016e+00, 1.00320376e+00,
       1.00460447e+00, 1.00657972e+00, 1.00932707e+00, 1.01307581e+00,
       1.01805918e+00, 1.02445794e+00, 1.03231568e+00, 1.04144744e+00,
       1.05139061e+00, 1.06144894e+00, 1.07083459e+00, 1.07884047e+00,
       1.08494561e+00, 1.08880020e+00, 1.09010270e+00, 1.08840519e+00,
       1.08285627e+00, 1.07184755e+00, 1.05251347e+00, 1.02013204e+00,
       9.67879872e-01, 8.88267024e-01, 7.77918313e-01, 6.43924924e-01,
       5.03921624e-01, 3.76562022e-01, 2.72384733e-01, 1.92999287e-01,
       1.35061486e-01, 9.38251491e-02, 6.48954574e-02, 4.47678113e-02,
       3.08326570e-02, 2.12133669e-02, 1.45854697e-02, 1.00240348e-02,
       6.88714850e-03, 4.73099342e-03, 3.24944011e-03, 2.23165086e-03,
       1.53256095e-03, 1.05242552e-03])),
 call.subplot().set_xlabel('Frequency (rad/sec)'),
 call.subplot().set_ylabel(''),
 call.subplot().grid(True, which='both'),
 call.tight_layout()]

Verifying this list should be suitable for unittesting.

Copy link
Contributor

Choose a reason for hiding this comment

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

Comparing with a fixed set of samples with values of arbitrary precision is equally brittle.

Copy link
Contributor

Choose a reason for hiding this comment

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

The issue to test here is not about the correct value of the curves but that the plot functions produce a non-crashing and legible plot.

Copy link
Contributor

Choose a reason for hiding this comment

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

@bnavigator
Respectfully disagree, we are not testing matplotlib, we must make sure that we send the right calls to matplotlib.

And we wouldn't compare the calls with a hard coded array, but with the data from within the gangof4 function itself, using float compares with tolerances.

I'll support your decision.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I still don't see a point in testing that the calls which are defined in the source code are the same calls which we retype again in the test code. There is nothing meaningful to compare.

@image_comparison(baseline_images=['gangof4-pvtol'], extensions=['png'])
def test_gangof4_pvtol(pvtol_inner):
plt.figure(num=None, figsize=(8, 6), dpi=80)
ctrl.gangof4(*pvtol_inner)
2 changes: 2 additions & 0 deletions setup.cfg
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,6 @@ universal=1
addopts = -ra
filterwarnings =
error:.*matrix subclass:PendingDeprecationWarning
markers =
style: avoid a warning message in matplotlib.testing.decorators