Unit tests for gangof4_plot

[murrayrm]

This PR adds a unit test for the gang of 4 plots, resolving issue #500.

It also tests whether the new GitHub Actions CI interface is working...

[coveralls]

Coverage increased (+1.004%) to 88.543% when pulling b300886 on murrayrm:unit_tests_gangof4 into 1e6da71 on python-control:master.

[bnavigator]

?

[murrayrm]

That's very odd. Should have been a copy of the image...

[murrayrm]

Symbolic link set up by pytest that apparently used an absolute path. Fixed.

[bnavigator]

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

[murrayrm]

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.

[bnavigator]

I hope we can get rid of Python 2 soon.

[henklaak]

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.

[henklaak]

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.

[bnavigator]

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

[bnavigator]

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.

[henklaak]

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

[bnavigator]

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.

[bnavigator]

• I don't think the symlinked file is needed at all.

• The comparison test fails on my machine:

expected:

actual:

diff:

Comes from float data, needs float tolerance?

E           matplotlib.testing.exceptions.ImageComparisonFailure: images not close (RMS 14.842):
E               result_images/freqresp_test/gangof4-pvtol.png
E               result_images/freqresp_test/gangof4-pvtol-expected.png
E               result_images/freqresp_test/gangof4-pvtol-failed-diff.png  

[murrayrm]

It looks like bitmap comparison is not going to work here (the problem is that small shifts are causing large differences in the bitmap). I'll have to write something that extracts individual elements and make sure they are in the right places.

[bnavigator]

I am not sure if it is really worth the effort. My suggestion was just an idea.

Would it suffice to

• remove the axis text by the appropriate keyword of the tester,
• switching off the grid, and
• a large enough resolution, (which would not increase the file size too much because the png compression should work good enough)?