Conda-forge version of slycot 0.3.3 generates errors #217

Activity

1. murrayrm

   added this to the 0.8.0 milestone on Jul 6, 2018
2. moorepants commented on Jul 6, 2018

   

   moorepants

   on Jul 6, 2018

   More actions

   This error occurs with conda forge's slycot 0.2.0, 0.3.0, and 0.3.3.
3. moorepants commented on Jul 6, 2018

   

   moorepants

   on Jul 6, 2018

   More actions

   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?
4. murrayrm commented on Jul 6, 2018

   

   murrayrm

   on Jul 6, 2018

   Last edited by murrayrm

   MemberAuthor

   More actions

   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).
5. murrayrm

   removed this from the 0.8.0 milestone on Jul 7, 2018
6. murrayrm commented on Jul 18, 2018

   

   murrayrm

   on Jul 18, 2018

   MemberAuthor

   More actions

   Fixed in #220.
7. murrayrm

   closed this as completedon Jul 18, 2018