xferfcn_test test_pole_mimo fails on arm and powerpc #343

Activity

1. bnavigator

   changed the title [-]xfercn_test test_pole_mimo fails on arm and powerpc[/-] [+]xferfcn_test test_pole_mimo fails on arm and powerpc[/+] on Nov 9, 2019
2. bnavigator commented on Nov 9, 2019

   

   bnavigator

   on Nov 9, 2019

   ContributorAuthor

   More actions

   Ignore my last question about slycot. It is not even used in _common_den().
   Only numpy.roots() involved in returning complex poles...
3. murrayrm commented on Nov 9, 2019

   

   murrayrm

   on Nov 9, 2019

   Member

   More actions

   Assuming that the problem comes down to a difference in precision between architectures, what is the best way to fix this? One possibility would be to introduce a configuration variable for the error tolerance that get used as a default and then this could be set in the test scripts for different architectures (?).
4. bnavigator commented on Nov 9, 2019

   

   bnavigator

   on Nov 9, 2019

   ContributorAuthor

   More actions

   numpy.finfo().eps as used in _common_den() should be enough to derive the necessary information.

   Have look at this one:

       def test_pole_mimo(self):
           """Test for correct MIMO poles."""
   
           sys = TransferFunction([[[1.], [1.]], [[1.], [1.]]],
                                  [[[1., 2.], [1., 3.]], [[1., 4., 4.], [1., 9., 14.]]])
           p = sys.pole()
   
           try:
               np.testing.assert_array_almost_equal(p, [-2., -2., -7., -3., -2.])
           except AssertionError as e:
               print("EPS: {}".format(np.finfo(float).eps))
               for i in [0, 1]:
                   for j in [0, 1]:
                       print("Roots of sys.den[{}][{}]:".format(i, j))
                       print(np.roots(sys.den[i][j]))
               numc, denc, denorderc = sys._common_den()
               print("Common Den:")
               print(denc)
               raise e

   [   78s] ...................F..........
   [   78s] ======================================================================
   [   78s] FAIL: Test for correct MIMO poles.
   [   78s] ----------------------------------------------------------------------
   [   78s] Traceback (most recent call last):
   [   78s]   File "/home/abuild/rpmbuild/BUILD/control-0.8.2/control/tests/xferfcn_test.py", line 426, in test_pole_mimo
   [   78s]     raise e
   [   78s] AssertionError: 
   [   78s] Arrays are not almost equal to 6 decimals
   [   78s] 
   [   78s] (shapes (6,), (5,) mismatch)
   [   78s]  x: array([-1.999985+0.000000e+00j, -2.000007+1.271532e-05j,
   [   78s]        -2.000007-1.271532e-05j, -7.      +0.000000e+00j,
   [   78s]        -3.      +0.000000e+00j, -2.      +0.000000e+00j])
   [   78s]  y: array([-2., -2., -7., -3., -2.])
   [   78s] -------------------- >> begin captured stdout << ---------------------
   [   78s] EPS: 2.22044604925e-16
   [   78s] Roots of sys.den[0][0]:
   [   78s] [-2.]
   [   78s] Roots of sys.den[0][1]:
   [   78s] [-3.]
   [   78s] Roots of sys.den[1][0]:
   [   78s] [-1.99999999 -2.00000001]
   [   78s] Roots of sys.den[1][1]:
   [   78s] [-7. -2.]
   [   78s] Common Den:
   [   78s] [[ 1.  6. 12.  8.]
   [   78s]  [ 1. 12. 41. 42.]]
   [   78s] 
   [   78s] --------------------- >> end captured stdout << ----------------------
   

   Maybe sqrt(eps) (or precision/2) would do the trick?

   But I still not fully understand that _common_den algorithm. How would it have to be modified to recognize den[1][0] as a polynomial with a double root?
5. murrayrm commented on Nov 9, 2019

   

   murrayrm

   on Nov 9, 2019

   Member

   More actions

   I'm not an expert on this, but my sense is that computing common denominators for MIMO transfer functions is often ill-conditioned. @repagh and @roryyorke are have more expertise here.
6. bnavigator commented on Nov 9, 2019

   

   bnavigator

   on Nov 9, 2019

   ContributorAuthor

   More actions

   Ok, @repagh, @roryyorke, please have a look at my take in #345. Changing threshold whether we already have the root or not to sqrt(eps) fixes the test failure.

   I also changed the logic to actually do something with imag_tol. That last warning in the function never would have triggered because den is only assigned polyfromroots().real further above.
7. murrayrm

   added this to the 0.8.3 milestone on Dec 29, 2019
8. murrayrm commented on Dec 30, 2019

   

   murrayrm

   on Dec 30, 2019

   Member

   More actions

   Fixed in PR #345
9. murrayrm

   closed this as completedon Dec 30, 2019