hmpf

114 views
Skip to first unread message

Martin R

unread,
Dec 2, 2025, 5:20:30 PMDec 2
to sage-devel
sage: OZ = lambda P: (ones_matrix(P.cardinality()) - P.lequal_matrix())
sage: set(min(OZ(P).eigenvalues()) for n in range(1, 7) for P in posets(n))
{-1, -1, 0}
sage: [type(x) for x in _]
[<class 'sage.rings.rational.Rational'>,
 <class 'sage.rings.rational.Rational'>,
 <class 'sage.rings.qqbar.AlgebraicNumber'>]

Martin R

unread,
Dec 3, 2025, 4:52:39 AMDec 3
to sage-devel
Do we really want that the parent of the result depends on the *value* of its input here, rather than on the parent of the input?

Actually, strictly speaking the documentation promises something else:

        Return a sequence of the eigenvalues of a matrix, with
        multiplicity. If the eigenvalues are roots of polynomials in ``QQ``,
        then ``QQbar`` elements are returned that represent each separate
        root.

Do I misunderstand this, or *is* the behaviour a bug?  (If so, it is easy to fix.)

I am guessing that harmonizing the hash of QQbar with the hash of QQ is expensive.  Also, I don't think it would be the correct fix.

Martin

Nils Bruin

unread,
Dec 3, 2025, 12:47:23 PMDec 3
to sage-devel
Your example hides a lesser consistency: the list of eigenvalues for a matrix over QQ does seem to lie either in Q (if all the eigenvalues lie in QQ) or in QQbar. Putting them in QQbar in all cases is of course the general thing to do, but if you know your matrix has rational eigenvalues and you just want to work with them (take their numerator and denominator perhaps? reduce them mod p?) it could be quite annoying to find them in QQbar. Arithmetic in QQbar is also going to be horribly slow in comparison.

I can see why, with such an expensive general parent, there  might be an argument to put the arguments in a smaller parent when possible (for all of them). There is a practical argument to be made here for why one might want to deviate from the purity of "parent of return value determined by parent of input". 

The documentation is formulated in a confusing way: It can be read as if for matrices over QQ, only *separate* roots are returned: no multiplicity information. I haven't tested this, but I'm pretty sure that's not what is intended.

The documentation also doesn't reflect the "parent optimization". I think that's your main question. As described above, I think there's some practical nuance to that...

In actuality, due to the problem "eigenvalues in what field"? I'd probably steer away from routines like "eigenvalues" in my own code, because I wouldn't dare to trust the choice that's made. I probably instead would just ask for the characteristic poly and determine its roots where I want (that's almost never QQbar unless it's in an interactive situation), and then work with that.

Vincent Delecroix

unread,
Dec 3, 2025, 1:53:54 PMDec 3
to sage-...@googlegroups.com
To my mind, it would be best to make eigenvalues actually have the
same behavior as polynomials

sage: QQ["x"]("x^2 + 1").roots() # only roots in the base ring
[]
sage: QQbar["x"]("x^2 + 1").roots()
[(-1*I, 1), (1*I, 1)]

Polynomials actually allow a nice syntax to search roots somewhere
else with the extra argument "ring"

sage: QQ["x"]("x^2 + 1").roots(QQbar)
[(-1*I, 1), (1*I, 1)]
sage: QQ["x"]("x^2 + 1").roots(ring=QQbar)
[(-1*I, 1), (1*I, 1)]

Vincent
> --
> You received this message because you are subscribed to the Google Groups "sage-devel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to sage-devel+...@googlegroups.com.
> To view this discussion visit https://groups.google.com/d/msgid/sage-devel/2a50d927-7b5b-47e1-b9d4-71074cc75155n%40googlegroups.com.

Dima Pasechnik

unread,
Dec 3, 2025, 2:24:38 PMDec 3
to sage-...@googlegroups.com


On December 3, 2025 12:51:20 PM CST, Vincent Delecroix <20100.d...@gmail.com> wrote:
>To my mind, it would be best to make eigenvalues actually have the
>same behavior as polynomials
>
>sage: QQ["x"]("x^2 + 1").roots() # only roots in the base ring
>[]
>sage: QQbar["x"]("x^2 + 1").roots()
>[(-1*I, 1), (1*I, 1)]
>
>Polynomials actually allow a nice syntax to search roots somewhere
>else with the extra argument "ring"
>
>sage: QQ["x"]("x^2 + 1").roots(QQbar)
>[(-1*I, 1), (1*I, 1)]
>sage: QQ["x"]("x^2 + 1").roots(ring=QQbar)
>[(-1*I, 1), (1*I, 1)]

+1 for adding "ring" option for eigenvalues/vectors

Martin R

unread,
Dec 4, 2025, 4:42:33 AMDec 4
to sage-devel
I don't think that in this case there would be a performance penality, the computation of the eigenvalues itself is not affected.  Regardless, I think that in general, a high level convenience method should be optimized for reliability, not for performance.  In fact, removing the optimization we could avoid the universe computation in eigenvalues = Sequence(res), which might be just as expensive.

Of course, if I know that the eigenvalues will be rational, it would be good to be able to tell the method that.  I am a bit unsure how difficult that would be.  I am guessing that the default ring cannot be the base ring, because that would break existing code, right?

Apart from that, I just found another snippet of the documentation, which says:

        The eigenvalues are elements of ``QQbar``, so they really represent
        exact roots of polynomials, not just approximations.

Here is what I would like to change:

diff --git a/src/sage/matrix/matrix2.pyx b/src/sage/matrix/matrix2.pyx
index 63102fb380b..0b3d2040e80 100644
--- a/src/sage/matrix/matrix2.pyx
+++ b/src/sage/matrix/matrix2.pyx
@@ -7297,7 +7297,7 @@ cdef class Matrix(Matrix1):
        res = []
        for f, e in self.charpoly().change_ring(K).factor():
            if f.degree() == 1:
-                res.extend([-f.constant_coefficient()]*e)
+                res.extend([A(-f.constant_coefficient())]*e)
            else:
                for r, ee in f.change_ring(A).roots():
                    res.extend([r]*(e*ee))

Vincent Delecroix

unread,
Dec 4, 2025, 4:58:41 AMDec 4
to sage-...@googlegroups.com
Note that what you propose Martin is also not backward compatible. It
can arguably be "less" backward compatible then my proposal. In terms
of performance, it is a bad idea to rely on generic factorization
algorithms to find rational roots of a polynomial (even though the
former might start by finding rational roots).

My concrete proposal is

def eivenvalues(self, ring=None, extends=False):
if ring is None:
ring = self.base_ring()
if extends:
ring = ring.algebraic_closure() # or possibly something smarter?
return self.charpoly().roots(ring)

(with appropriate deprecation warnings of course)

Is there any reason why this would not be the best option (beyond
annoying backward compatibility issues)?
> --
> You received this message because you are subscribed to the Google Groups "sage-devel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to sage-devel+...@googlegroups.com.
> To view this discussion visit https://groups.google.com/d/msgid/sage-devel/6f4f4480-6628-4aa4-945d-6c4894c1649cn%40googlegroups.com.

Nils Bruin

unread,
Dec 4, 2025, 12:24:44 PMDec 4
to sage-devel
On Thursday, 4 December 2025 at 01:58:41 UTC-8 vdelecroix wrote:
My concrete proposal is

def eivenvalues(self, ring=None, extends=False):
if ring is None:
ring = self.base_ring()
if extends:
ring = ring.algebraic_closure() # or possibly something smarter?
return self.charpoly().roots(ring) 

I think you'd end up a little more backwards compatible by making the default for base ring QQ to be "extends=True".  Otherwise that seems like a flexible solution to me. In fact, there is already an extend option to the command! Adding the "ring" parameter would be a change. The default is "extend=True", so Martin's request is really just to return results in QQbar always when "extend=True". That's OK, I think. We should check that with "extend=False" we do return the roots just in the base ring. The keyword argument is already documented ...



Dima Pasechnik

unread,
Dec 4, 2025, 12:57:10 PMDec 4
to sage-...@googlegroups.com
I think "extend=" also can take a concrete field as an argument, as there are cases when you know what it can be, and e.g. number fields are faster than QQbar.
(or, for instance, you know that the eigenvalues are real, and you want to pass this on)

John Cremona

unread,
Dec 5, 2025, 6:21:26 AMDec 5
to sage-devel
I remember that there was a long discussion about this issue many years ago (Nils may remember).  At that time the current behavious was decided upon.  One aspect which seemed important at the time is not to confuse unsophisticated users, for example students in first linear algebra classes, who could be put off my being told that M = Matrix([[0,2],[1,0]]); M.eigenvalues() returned an empty list while they were expecting +sqrt(2), -sqrt(2).  The compromise was to use QQbar here, so that returned  [-1.414213562373095?, 1.414213562373095?] looks like what they would expect to see (apart from the ? marks perhaps) and can be further worked with, while experts can do more, such as coercing them into number field elements or RR or CC etc.

So I would hesitate before switching to a position where only eigenvalues in the base ring are returned unless a non-default paramater option is provided.  On the other hand, I would support having a ring parameter which if None (the default) would revert to the current behavious over QQ.  Then there would be no change for beginners, while others would be able to say M.eigenvalues(ring=RR) or M.eigenvalues(ring=my_favourite_number_field).

For consistency we should probably do the same thing for roots of polynomials in QQ[x].  WIth the above M, M.charpoly().roots() returns [],  not the same as M.eigenvalues()!  If we were to change roots() (for univariate polynomials over QQ only) to match the eigenvalues() behaviour it could return roots in QQbar unless you specifically set ring=QQ.

John

Nils Bruin

unread,
Dec 5, 2025, 11:48:27 AMDec 5
to sage-devel
On Friday, 5 December 2025 at 03:21:26 UTC-8 john.c...@gmail.com wrote:
So I would hesitate before switching to a position where only eigenvalues in the base ring are returned unless a non-default paramater option is provided.  On the other hand, I would support having a ring parameter which if None (the default) would revert to the current behavious over QQ.  Then there would be no change for beginners, while others would be able to say M.eigenvalues(ring=RR) or M.eigenvalues(ring=my_favourite_number_field).

For consistency we should probably do the same thing for roots of polynomials in QQ[x].  WIth the above M, M.charpoly().roots() returns [],  not the same as M.eigenvalues()!  If we were to change roots() (for univariate polynomials over QQ only) to match the eigenvalues() behaviour it could return roots in QQbar unless you specifically set ring=QQ.

Thank you, John, for bringing up this perspective. I think the argument for "eigenvalues" is a good one. I wouldn't generalize to roots of polynomials, though: that a polynomial over QQ "has no roots" when it is irreducible (and of degree at least 2) is also taught quite early. So I think the default behaviour of "roots" is fine. The difference with what "eigenvalues" does is up to "eigenvalues" to explain.

(Also, "roots" is quite essential in serious code -- one should be careful in compromising it with features for interactive and educational use. That might have to go in a wrapper instead, if it is that badly needed) 
Reply all
Reply to author
Forward
0 new messages