Fix #38578 CI failure

[grhkm21]

As title. It seems the issue is because the eigenvalues might duplicate causing the eigenspaces to be of unexpected size, so I replaced the [ZZ.random_element() for _ in range(3)] with sample(range(-20, 21), 3).

Fixes #38578

[github-actions]

Documentation preview for this PR (built with commit 4a59ec1; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

[kwankyu]

Even with distinct eigenvalues, I don't see how it is guaranteed that

        sage: B = random_matrix(QQ, 6, algorithm='diagonalizable',
        ....:                   eigenvalues=eigenvalues, dimensions=[2,3,1])
        sage: all(x in ZZ for x in (B-(-12*identity_matrix(6))).rref().list())
        True
        sage: all(x in ZZ for x in (B-(4*identity_matrix(6))).rref().list()); B
        True
        sage: all(x in ZZ for x in (B-(6*identity_matrix(6))).rref().list())
        True  

It seems that the author of this doctest chose -12, 4, 6 eigenvalues. Even with this case, how can we be sure that the reduced row echelon form of the matrix B-(4*identity_matrix(6)) consists of integer entries? I have no idea...

[kwankyu]

For your convenience, this is the failing case:

sage: B = Matrix(QQ, [
....:     [-44, -72, 64, -224, -432, 184],
....:     [36, 24, -20, 156, 184, -148],
....:     [52, 56, -28, 180, 328, -236],
....:     [16, 16, -12, 68, 104, -68],
....:     [0, 4, 0, -8, 12, -4],
....:     [4, 0, 4, 4, 0, -20]
....: ])
sage: B
[ -44  -72   64 -224 -432  184]
[  36   24  -20  156  184 -148]
[  52   56  -28  180  328 -236]
[  16   16  -12   68  104  -68]
[   0    4    0   -8   12   -4]
[   4    0    4    4    0  -20]
sage: B.diagonalization()
(
[4 0 0 0 0 0]  [       1        0        0        1        0        0]
[0 4 0 0 0 0]  [       0        1        0        0        1        0]
[0 0 4 0 0 0]  [       0        0        1        0        0        1]
[0 0 0 0 0 0]  [   -5/62   27/124   11/124    -1/14      1/7      1/7]
[0 0 0 0 0 0]  [  -1/248 -131/496   89/496     1/70    -8/35     6/35]
[0 0 0 0 0 0], [  19/124    9/248   45/248    13/70     1/35     8/35]
)
sage: D, P = _
sage: P^-1*B*P == D
True
sage: X = B-(4*identity_matrix(6))
sage: X
[ -48  -72   64 -224 -432  184]
[  36   20  -20  156  184 -148]
[  52   56  -32  180  328 -236]
[  16   16  -12   64  104  -68]
[   0    4    0   -8    8   -4]
[   4    0    4    4    0  -24]
sage: X.rref()
[    1     0     0  27/7  18/7 -31/7]
[    0     1     0    -2     2    -1]
[    0     0     1 -20/7 -18/7 -11/7]
[    0     0     0     0     0     0]
[    0     0     0     0     0     0]
[    0     0     0     0     0     0]

[grhkm21]

Hmm yes, I don’t know what the test is trying to check for either. I just saw that in the failing test case the eigenvalues chosen were [4,0,4] so I guessed that was the issue. Should we just remove it

…

On 29 Aug 2024 at 15:04 +0100, Kwankyu Lee ***@***.***>, wrote: For your convenience, this is the failing case: sage: B = Matrix(QQ, [ ....: [-44, -72, 64, -224, -432, 184], ....: [36, 24, -20, 156, 184, -148], ....: [52, 56, -28, 180, 328, -236], ....: [16, 16, -12, 68, 104, -68], ....: [0, 4, 0, -8, 12, -4], ....: [4, 0, 4, 4, 0, -20] ....: ]) sage: B [ -44 -72 64 -224 -432 184] [ 36 24 -20 156 184 -148] [ 52 56 -28 180 328 -236] [ 16 16 -12 68 104 -68] [ 0 4 0 -8 12 -4] [ 4 0 4 4 0 -20] sage: B.diagonalization() ( [4 0 0 0 0 0] [ 1 0 0 1 0 0] [0 4 0 0 0 0] [ 0 1 0 0 1 0] [0 0 4 0 0 0] [ 0 0 1 0 0 1] [0 0 0 0 0 0] [ -5/62 27/124 11/124 -1/14 1/7 1/7] [0 0 0 0 0 0] [ -1/248 -131/496 89/496 1/70 -8/35 6/35] [0 0 0 0 0 0], [ 19/124 9/248 45/248 13/70 1/35 8/35] ) sage: D, P = _ sage: P^-1*B*P == D True sage: X = B-(4*identity_matrix(6)) sage: X [ -48 -72 64 -224 -432 184] [ 36 20 -20 156 184 -148] [ 52 56 -32 180 328 -236] [ 16 16 -12 64 104 -68] [ 0 4 0 -8 8 -4] [ 4 0 4 4 0 -24] sage: X.rref() [ 1 0 0 27/7 18/7 -31/7] [ 0 1 0 -2 2 -1] [ 0 0 1 -20/7 -18/7 -11/7] [ 0 0 0 0 0 0] [ 0 0 0 0 0 0] [ 0 0 0 0 0 0] — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.Message ID: ***@***.***>

[kwankyu]

We may remove the checking part, which is vague and not explicitly supported by random_matrix(..., algorithm='diagonalizable', ...)

Wait... random_diagonalizable_matrix states something about integers...

[kwankyu]

The doc of random_diagonalizable_matrix says that the eigenspaces of the generated matrix have basis vectors with only integer entries. This is always true for any diagonalizable matrix over rationals with rational eigenvalues...

Perhaps check `all(x in QQ for x in (B-(y*identity_matrix(6))).list()) for eigenvalues y = -12, 4, 6?

[user202729]

It says something more specific:

To be used as a teaching tool. Return matrices have only real eigenvalues.

and

A square, diagonalizable, matrix with only integer entries. The eigenspaces of this matrix, if computed by hand, give basis vectors with only integer entries.

Of course given basis vectors with rational entries you can multiply it by the common denominator (by hand or not) to get integer entries. But I think the intention is, given eigenvalue λ, you can compute the basis vector of the eigenspace of λ by hand as follows:

• compute M-λ I.
• Find a basis for the null space of that matrix, which is done by:
  • compute rref of that matrix.
  • for each non-pivot column,
    • set the value in that column to be 1
    • set the values in other non-pivot columns to be 0
    • compute the values in the pivot columns using the rref computed above

to make hand computation easy, it's better if the rref has only integer entries. That's why the rref is computed, I suppose.

In any case, if the implementation does guarantee the rref has only integer entries I don't see any problem with keeping the test.

[kwankyu]

In any case, if the implementation does guarantee the rref has only integer entries I don't see any problem with keeping the test.

No problem with me if so. But it is hard for me to imagine that it (rref has only integer entries) is guaranteed if the eigenvalues are distinct and not otherwise.

[user202729]

(for crosslink purpose) Also this fixes #38578. Apparently the #38578 in the issue title are not detected.

[grhkm21]

Oops sorry I was inactive the past few days.

But I think the intention is, given eigenvalue λ, you can compute the basis vector of the eigenspace of λ by hand as follows: ...

I understand that part, but I don't understand why the test would be correct. I tried understanding the code but idk what all the dimension_check thing is doing. However, there's definitely something special about the algorithm. In fact the test that I patch in this PR works in general, but I don't know why it doesn't in #38579.

sage: ev = sorted(sample(range(-20, 21), 6))
....: D = diagonal_matrix(ev)
....: P = random_matrix(ZZ, 6, 6, algorithm="unimodular")
....: M = P * D * P^-1
....: for e in ev:
....:     print(set((M - e).rref().list()))
{0, 1, -2/3, 1/3, 4/3}
{0, 1, 4/7, 2/7, 3/14, -1/7}
{0, 1, 22/89, 55/89, 32/89, 4/89, -25/89}
{0, 1, 120/401, -95/401, 105/401, 11/401, 237/401}
{0, 1, 267/991, 273/991, 20/991, -224/991, 578/991}
{0, 1, 59/233, 65/233, 3/233, 135/233, -48/233}

....: for _ in trange(10^5):
....:     ev = sample(range(-20, 21), 6)
....:     M = random_matrix(ZZ, 6, algorithm="diagonalizable", eigenvalues=ev, dimensions=[1] * 6)
....:     # for e in ev:
....:     #     print(set((M - e).rref().list()))
....:     assert all(u in ZZ for e in ev for u in (M - e).rref().list())  # passes 10000 times
sage: for e in ev:
....:     print(set((M - e).rref().list()))
{0, 1, 3, 4, -3, -1}
{0, 1, 3, 4, -1, -2}
{0, 1, 3, 4, -1, -2}
{0, 1, 3, 4, -1, -2}
{0, 1, 2, 4, -1, -2}
{0, 1, 3, -1, -2}

[grhkm21]

(for crosslink purpose) Also this fixes #38578. Apparently the #38578 in the issue title are not detected.

Thanks, edited the PR description.