Skip to content

Conda-forge version of slycot 0.3.3 generates errors #217

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

Closed
murrayrm opened this issue Jul 6, 2018 · 4 comments
Closed

Conda-forge version of slycot 0.3.3 generates errors #217

murrayrm opened this issue Jul 6, 2018 · 4 comments

Comments

@murrayrm
Copy link
Member

murrayrm commented Jul 6, 2018

When installing slycot from conda-forge (v0.3.3), I get the following error when running python setup.py test:

FAIL: testMinrealBrute (control.tests.minreal_test.TestMinreal)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/murray/Dropbox/macosx/src/python-control/murrayrm/control/tests/minreal_test.py", line 62, in testMinrealBrute
    raise e
  File "/Users/murray/Dropbox/macosx/src/python-control/murrayrm/control/tests/minreal_test.py", line 57, in testMinrealBrute
    ht1.den[0][0], ht2.den[0][0])
  File "/Users/murray/Dropbox/macosx/src/python-control/murrayrm/control/tests/minreal_test.py", line 35, in assert_numden_almost_equal
    np.testing.assert_array_almost_equal(n1, n2)
  File "/Users/murray/anaconda/envs/python3.6-oldslycot/lib/python3.6/site-packages/numpy/testing/nose_tools/utils.py", line 963, in assert_array_almost_equal
    precision=decimal)
  File "/Users/murray/anaconda/envs/python3.6-oldslycot/lib/python3.6/site-packages/numpy/testing/nose_tools/utils.py", line 717, in assert_array_compare
    raise AssertionError(msg)
AssertionError: 
Arrays are not almost equal to 6 decimals

(shapes (3,), (4,) mismatch)
 x: array([ -1.33214 , -10.368378, -13.703545])
 y: array([ -1.33214 , -13.297947, -36.505109, -30.136079])

This error does not occur when using pip version of slycot. Need to understand what is going on before releasing 0.8.0.

Note: conda-forge version of slycot uses OpenBLAS, pip version uses lapack (I think).

@moorepants
Copy link

This error occurs with conda forge's slycot 0.2.0, 0.3.0, and 0.3.3.

@moorepants
Copy link

How precise is this test? I can imagine minreal being finicky because of floating point operations. How robust is it to different platforms, architectures, and blas/lapack implementations?

@murrayrm
Copy link
Member Author

murrayrm commented Jul 6, 2018

Good to know that this is not a new bug. I was pretty sure I had seen the error message before, but couldn't figure out where.

I chased this down a bit. It looks like what is happening is that one of the (random) state space systems that is getting generated in the unit test has a near pole-zero cancellation. minreal() is not reducing the system (via a call to tb01pd ). But when the unit test checks to make sure minreal is not changing the system, it converts the state space system to a transfer function and one of systems is now reduced (inside of tb04ad, where it extracts the controllable-observable subsystem). This is probably just a difference in tolerances, as @moorepants indicated. Interestingly, the error doesn't show up on my local system if I use the lapack variant of slycot instead of the openblas variant.

Here are the systems that are being compared:

System 1 (original):

A = [[-2.07272289  0.16120168  0.05029039 -0.17656953]
 [-1.2172162  -3.0412386   0.87634026  0.45936988]
 [ 2.34727018  2.1024825  -2.97194391 -1.72217184]
 [ 2.70301798  2.1897608  -1.33329543 -3.77801591]]

B = [[ 0.22934264]
 [-1.05015731]
 [-0.40877983]
 [-1.282876  ]]

C = [[-1.22961654  0.9999803  -0.         -0.        ]]

D = [[0.]]

   -1.332 s^2 - 10.37 s - 13.7
---------------------------------
s^3 + 9.665 s^2 + 26.21 s + 21.53

Zeros =  [-6.09568359 -1.68756532]
Poles =  [-5.76863334 -2.1991448  -1.69699836]

System 2 (output of minreal):

A = [[-1.44793116  0.00720403 -1.02465799 -0.32451831]
 [ 1.29258628 -2.23726554 -1.36828447 -1.39979547]
 [ 2.18253992 -0.07706622 -4.41025255 -2.57508146]
 [ 1.75507481 -0.03186882 -2.0133189  -3.76847205]]

B = [[-1.09979463]
 [ 0.78245651]
 [-0.96370783]
 [-0.46667011]]

C = [[ 0.         -1.46759237 -0.09533846  0.59076278]]

D = [[0.]]

   -1.332 s^3 - 13.3 s^2 - 36.51 s - 30.14
---------------------------------------------
s^4 + 11.86 s^3 + 47.46 s^2 + 79.16 s + 47.34

Zeros =  [-6.09568359 -2.1991448  -1.68756532]
Poles =  [-5.76863334+0.00000000e+00j -2.1991448+2.29466803e-07j
 -2.1991448-2.29466803e-07j -1.69699836+0.00000000e+00j]

The cancellation is occurring in the first system but not the second. In particular, in the second system there is a zero at -2.1991448 and a pair of poles at -2.1991448 +/- 2.29466803e-07j (one of which gets apparently gets cancelled in the first system).

There are two issues here:

  • The criterion in minreal for a pole/zero cancellation is different than in ss2tf.
  • The minreal unit test is not testing (just) minreal, but is also (implicitly) testing th ss2tf conversion.

Not sure yet what the right fix is (if any).

@murrayrm
Copy link
Member Author

Fixed in #220.

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

No branches or pull requests

2 participants