Add doctest for eigenmatrix of complex floating-point matrix #12595

dkrenn · 2012-02-26T19:18:38Z

sage: m = Matrix(CDF, 8, [[-1, -1, -1, -1, 1, -3, -1, -1], [1, 1, 1, 1, -1, -1, 1, -3], [-1, 3, -1, -1, 1, 1, -1, -1], [-1, -1, -1, 3, 1, 1, -1, -1], [1, 1, -3, 1, -1, -1, 1, 1], [1, 1, 1, 1, -1, -1, -3, 1], [3, -1, -1, -1, 1, 1, -1, -1], [1, 1, 1, 1, 3, -1, 1, 1]])
sage: d, p = m.eigenmatrix_left()
sage: (p[1] * m)[0] / p[1][0]
1.2360679775 - 3.80422606518*I
sage: d[1][1]
1.2360679775 + 3.80422606518*I

Sage seems to return the complex conjugate of d or something of the sort. Perhaps d is simply wrongly permuted (but real eigenvalues seem to be correct).

p.inverse() * d * p should be at least approximately equal to m.

This was reported on the public bug reports from the notebook interface by david+bugs@madore.org on 1/24/2012.

CC: @rbeezer @slel

Component: linear algebra

Keywords: eigenmatrix CDF sd40.5

Stopgaps: todo

Author: Shashwat Singh

Branch/Commit: c7e4e78

Reviewer: Samuel Lelièvre

Issue created by migration from https://trac.sagemath.org/ticket/12595

The text was updated successfully, but these errors were encountered:

sagetrac-dsm · 2012-05-27T23:44:16Z

comment:2

Somewhat smaller example, and evidence that something's wrong in RDF too:


sage: M = [[1,-1],[1, 1]]
sage: rings = SR, ZZ, QQ, RDF, CDF
sage: for ring in rings:
....:     m = Matrix(ring, M)
....:     d, p = m.left_eigenmatrix()
....:     print m- (~p)*d*p
....: 
[0 0]
[0 0]
[0 0]
[0 0]
[0 0]
[0 0]
[ 0.0 -2.0]
[ 2.0  0.0]
[2.22044604925e-16 + 1.96068977308e-16*I              -2.0 - 2.22044604925e-16*I]
[              2.0 + 8.14867788484e-17*I                      -2.22044604925e-16]

sagetrac-dsm · 2012-05-27T23:47:13Z

comment:3

[Of course even if m is over RDF d and p can be over CDF.]

rbeezer · 2012-05-28T03:06:15Z

comment:4

Appears the computed results in both of the above examples are correct for the transpose of the matrix. Digging deeper.......

rbeezer · 2012-05-28T03:06:15Z

Changed keywords from eigenmatrix, CDF to eigenmatrix, CDF, sd40.5

rbeezer · 2012-05-28T03:06:50Z

Changed keywords from eigenmatrix, CDF, sd40.5 to eigenmatrix CDF sd40.5

dkrenn · 2015-04-16T20:32:45Z

comment:10

Still getting

sage: norm(p.inverse() * d * p - m)
7.608452130361234

on Sage 6.6.

FYI, doing the same but over QQbar gives 1.3060006046871026e-37.

sagetrac-jakobkroeker · 2015-08-25T17:45:56Z

Stopgaps: todo

shashwat1002 · 2022-02-12T19:33:56Z

comment:12

I think this is no longer a problem

sage: m = Matrix(CDF, 8, [[-1, -1, -1, -1, 1, -3, -1, -1], [1, 1, 1, 1, -1, -1, 1, -3], [-1, 3, -1, -1, 1, 1, -1, -1], [-1, -1, -1, 3, 1, 1, -1, -1], [1, 1, -3, 1, -1, -1, 1, 1], [1, 1, 1, 1, -1, -1, -3, 1], [3, -1, -1, -1, 1, 1, -1, -1], [1, 1, 1, 1, 3, -1, 1, 1]])
sage: d, p = m.eigenmatrix_left()
sage: norm(p.inverse() * d * p - m)
8.505150621424852e-15

The answer for the norm is now close enough to 0 so probably the underlying issue has been resolved.

slel · 2022-02-12T23:05:49Z

comment:14

Could a doctest be added for that?

shashwat1002 · 2022-02-13T07:46:37Z

Author: Shashwat Singh

shashwat1002 · 2022-02-13T07:46:37Z

Commit: d1ed045

shashwat1002 · 2022-02-13T07:46:37Z

Branch: u/gh-shashwat1002/wrong-eigenmatrix

shashwat1002 · 2022-02-13T07:46:37Z

New commits:

`d1ed045`	`Trac #12595: elgennmatrix of complex floating-point matrix`

shashwat1002 · 2022-02-13T07:49:02Z

comment:16

Replying to @slel:

Could a doctest be added for that?

Just added

The doctest tests if it is within 10^(-12)

That precision has been chosen arbitrarily.

I hope this suffices.

slel · 2022-02-13T10:55:16Z

comment:17

Currently your commit looks like this
(where _ indicates trailing whitespace):

$ git show
commit d1ed045943d68...
Author: ...
Date:   ...

    Trac #12595: elgennmatrix of complex floating-point matrix

    Add doctest to demonstrate that the issue is already resolved

diff --git a/src/sage/matrix/matrix2.pyx b/src/sage/matrix/matrix2.pyx
index 7c5cd644d4..3ed18eb108 100644
--- a/src/sage/matrix/matrix2.pyx
+++ b/src/sage/matrix/matrix2.pyx
@@ -6936,6 +6936,14 @@ cdef class Matrix(Matrix1):
             Traceback (most recent call last):
             ...
             RuntimeError: failed to compute eigenvectors for eigenvalue ..., check eigenvectors_left() for partial results
+________
+        The following example shows that :trac:`12595` has been resolved_
+____________
+            sage: m = Matrix(CDF, 8, [[-1, -1, -1, -1, 1, -3, -1, -1], [1, 1, 1, 1, -1, -1, 1, -3], [-1, 3, -1, -1, 1, 1, -1, -1], [-1, -1, -1, 3, 1, 1, -1, -1], [1, 1, -3, 1, -1, -1, 1, 1], [1, 1, 1, 1, -1, -1, -3, 1], [3, -1, -1, -1, 1, 1, -1, -1], [1, 1, 1, 1, 3, -1, 1, 1]])
+            sage: d, p = m.eigenmatrix_left()
+            sage: norm(p.inverse() * d * p - m) < 10^(-12)
+            True
+
         """
         from sage.misc.flatten import flatten
         from sage.matrix.constructor import diagonal_matrix, matrix

Comments:

the commit message has "elgenmatrix" for "eigenmatrix"
several lines have trailing white space, please remove it
remove the blank line at the end of the docstring,
between the test and the closing """
the line "The following ... resolved" should end with ::
to actually get tested
please use 1e-12 instead of 10^-12 here
the example input could be wrapped as follows

            sage: m = Matrix(CDF, 8, [[-1, -1, -1, -1, 1, -3, -1, -1],
            ....:                     [1, 1, 1, 1, -1, -1, 1, -3],
            ....:                     [-1, 3, -1, -1, 1, 1, -1, -1],
            ....:                     [-1, -1, -1, 3, 1, 1, -1, -1],
            ....:                     [1, 1, -3, 1, -1, -1, 1, 1],
            ....:                     [1, 1, 1, 1, -1, -1, -3, 1],
            ....:                     [3, -1, -1, -1, 1, 1, -1, -1],
            ....:                     [1, 1, 1, 1, 3, -1, 1, 1]])

Finally, I think splitting the last input line as follows
would make the test even clearer:

            sage: mm = p.inverse() * d * p  # equals `m` up to numerical noise
            sage: (mm - m).norm() < 1e-12

Can you take all these comments into account and send a new commit?
You can force-push to replace the previous commit.

slel · 2022-02-13T10:55:16Z

Reviewer: Samuel Lelièvre

sagetrac-git · 2022-02-13T14:45:06Z

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

`6e73228`	`Trac #12595: eigenmatrix_left of complex floating point matrix`

sagetrac-git · 2022-02-13T14:45:06Z

Changed commit from d1ed045 to 6e73228

shashwat1002 · 2022-02-13T14:46:14Z

comment:19

Replying to @slel:

Currently your commit looks like this
(where _ indicates trailing whitespace):

$ git show
commit d1ed045943d68...
Author: ...
Date:   ...

    Trac #12595: elgennmatrix of complex floating-point matrix

    Add doctest to demonstrate that the issue is already resolved

diff --git a/src/sage/matrix/matrix2.pyx b/src/sage/matrix/matrix2.pyx
index 7c5cd644d4..3ed18eb108 100644
--- a/src/sage/matrix/matrix2.pyx
+++ b/src/sage/matrix/matrix2.pyx
@@ -6936,6 +6936,14 @@ cdef class Matrix(Matrix1):
             Traceback (most recent call last):
             ...
             RuntimeError: failed to compute eigenvectors for eigenvalue ..., check eigenvectors_left() for partial results
+________
+        The following example shows that :trac:`12595` has been resolved_
+____________
+            sage: m = Matrix(CDF, 8, [[-1, -1, -1, -1, 1, -3, -1, -1], [1, 1, 1, 1, -1, -1, 1, -3], [-1, 3, -1, -1, 1, 1, -1, -1], [-1, -1, -1, 3, 1, 1, -1, -1], [1, 1, -3, 1, -1, -1, 1, 1], [1, 1, 1, 1, -1, -1, -3, 1], [3, -1, -1, -1, 1, 1, -1, -1], [1, 1, 1, 1, 3, -1, 1, 1]])
+            sage: d, p = m.eigenmatrix_left()
+            sage: norm(p.inverse() * d * p - m) < 10^(-12)
+            True
+
         """
         from sage.misc.flatten import flatten
         from sage.matrix.constructor import diagonal_matrix, matrix

Comments:

the commit message has "elgenmatrix" for "eigenmatrix"
several lines have trailing white space, please remove it
remove the blank line at the end of the docstring,
between the test and the closing """
the line "The following ... resolved" should end with ::
to actually get tested
please use 1e-12 instead of 10^-12 here
the example input could be wrapped as follows

            sage: m = Matrix(CDF, 8, [[-1, -1, -1, -1, 1, -3, -1, -1],
            ....:                     [1, 1, 1, 1, -1, -1, 1, -3],
            ....:                     [-1, 3, -1, -1, 1, 1, -1, -1],
            ....:                     [-1, -1, -1, 3, 1, 1, -1, -1],
            ....:                     [1, 1, -3, 1, -1, -1, 1, 1],
            ....:                     [1, 1, 1, 1, -1, -1, -3, 1],
            ....:                     [3, -1, -1, -1, 1, 1, -1, -1],
            ....:                     [1, 1, 1, 1, 3, -1, 1, 1]])

Finally, I think splitting the last input line as follows
would make the test even clearer:

            sage: mm = p.inverse() * d * p  # equals `m` up to numerical noise
            sage: (mm - m).norm() < 1e-12

Can you take all these comments into account and send a new commit?
You can force-push to replace the previous commit.

Just did, please check :)

slel · 2022-02-13T16:01:24Z

comment:20

Please

remove trailing whitespace in the definition of m,
remove blank line between definition of m and of d, p
put two spaces before # for the inline comment

sagetrac-git · 2022-02-13T16:48:10Z

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

`c7e4e78`	`Trac #12595: eigenmatrix_left of complex floating point matrix`

sagetrac-git · 2022-02-13T16:48:10Z

Changed commit from 6e73228 to c7e4e78

shashwat1002 · 2022-02-13T16:49:19Z

comment:22

Replying to @slel:

Please

remove trailing whitespace in the definition of m,

remove blank line between definition of m and of d, p

put two spaces before # for the inline comment

I have dealt with the white discrepancies.

Please check :)

slel · 2022-02-13T21:56:52Z

comment:23

Looks good to me.

vbraun · 2022-02-20T13:27:58Z

Changed branch from u/gh-shashwat1002/wrong-eigenmatrix to c7e4e78

dkrenn added this to the sage-5.11 milestone Feb 26, 2012

dkrenn added c: linear algebra labels Feb 26, 2012

dkrenn assigned jasongrout and williamstein Feb 26, 2012

jdemeyer modified the milestones: sage-5.11, sage-5.12 Aug 13, 2013

sagetrac-vbraun-spam mannequin modified the milestones: sage-6.1, sage-6.2 Jan 30, 2014

sagetrac-vbraun-spam mannequin modified the milestones: sage-6.2, sage-6.3 May 6, 2014

sagetrac-vbraun-spam mannequin modified the milestones: sage-6.3, sage-6.4 Aug 10, 2014

shashwat1002 mannequin added the s: needs review label Feb 12, 2022

slel added s: needs info and removed s: needs review labels Feb 12, 2022

slel modified the milestones: sage-6.4, sage-9.6 Feb 12, 2022

shashwat1002 mannequin added s: needs review and removed s: needs info labels Feb 13, 2022

slel changed the title ~~eigenmatrix of complex floating-point matrix is wrong~~ Add doctest for eigenmatrix of complex floating-point matrix Feb 13, 2022

slel added s: positive review and removed s: needs review labels Feb 13, 2022

vbraun removed the s: positive review label Feb 20, 2022

vbraun closed this as completed in ff40ba0 Feb 20, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add doctest for eigenmatrix of complex floating-point matrix #12595

Add doctest for eigenmatrix of complex floating-point matrix #12595

dkrenn commented Feb 26, 2012

sagetrac-dsm mannequin commented May 27, 2012

sagetrac-dsm mannequin commented May 27, 2012

rbeezer mannequin commented May 28, 2012

rbeezer mannequin commented May 28, 2012

rbeezer mannequin commented May 28, 2012

dkrenn commented Apr 16, 2015

sagetrac-jakobkroeker mannequin commented Aug 25, 2015

shashwat1002 mannequin commented Feb 12, 2022

slel commented Feb 12, 2022

shashwat1002 mannequin commented Feb 13, 2022

shashwat1002 mannequin commented Feb 13, 2022

shashwat1002 mannequin commented Feb 13, 2022

shashwat1002 mannequin commented Feb 13, 2022

shashwat1002 mannequin commented Feb 13, 2022

slel commented Feb 13, 2022

slel commented Feb 13, 2022

sagetrac-git mannequin commented Feb 13, 2022

sagetrac-git mannequin commented Feb 13, 2022

shashwat1002 mannequin commented Feb 13, 2022

slel commented Feb 13, 2022

sagetrac-git mannequin commented Feb 13, 2022

sagetrac-git mannequin commented Feb 13, 2022

shashwat1002 mannequin commented Feb 13, 2022

slel commented Feb 13, 2022

vbraun commented Feb 20, 2022