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)?
On Thu, 4 Dec 2025 at 10:42, 'Martin R' via sage-devel
<[email protected]> wrote:
>
> 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))
>
> On Wednesday, 3 December 2025 at 18:47:23 UTC+1 Nils Bruin wrote:
>>
>> 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.
>>
>>
>> On Wednesday, 3 December 2025 at 01:52:39 UTC-8 [email protected] wrote:
>>>
>>> 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
>>> On Tuesday, 2 December 2025 at 23:20:30 UTC+1 Martin R wrote:
>>>>
>>>> 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'>]
>>>>
> --
> 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 [email protected].
> To view this discussion visit
> https://groups.google.com/d/msgid/sage-devel/6f4f4480-6628-4aa4-945d-6c4894c1649cn%40googlegroups.com.
--
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 [email protected].
To view this discussion visit
https://groups.google.com/d/msgid/sage-devel/CAGEwAAn3ckmKfw_rKGebii4GGwAJA3Sa2V8GvZp%3DMDLEKN3wQg%40mail.gmail.com.
