JuliaPy / SymPy.jl

Julia interface to SymPy via PyCall
http://juliapy.github.io/SymPy.jl/
MIT License
268 stars 62 forks source link

ordering of eigvals/eigvecs inconsistent with LinearAlgebra #495

Closed aosidnf closed 1 year ago

aosidnf commented 1 year ago

Specifically, the ordering is inconsistent such that it makes the common diagonalization behavior not work:

using LinearAlgebra
A=rand(2,2)
ξ=eigvecs(A)
λ=eigvals(A)
ξ*Diagonal(λ)*inv(ξ) -A # returns something close to zero ✅

but for a matrix of Syms,

using SymPy
As=Sym["a" "b"; "c" "d"]
ξs=eigvecs(As)
λs=eigvals(As)
ξs*Diagonal(λs)*inv(ξs) - As .|> simplify # returns something which isn't zero ❌
ξs*Diagonal([ λs[2],λs[1] ])*inv(ξs) - As .|> simplify # returns zero
mzaffalon commented 1 year ago

It works for me with SymPy v1.1.8 and Julia v1.8.5

using SymPy, LinearAlgebra
As=Sym["a" "b"; "c" "d"];
ξs=eigvecs(As);
λs=eigvals(As);
ξs*Diagonal(λs)*inv(ξs) - As .|> simplify # gives a 2x2 zero matrix
aosidnf commented 1 year ago

I am on the same version of SymPy and Julia. this zip contains a nix shell and project.toml which produces the issue for me. https://files.catbox.moe/7vkg5s.zip

mzaffalon commented 1 year ago

Can you try a different shell?

jverzani commented 1 year ago

The order is inherited from the python code https://github.com/JuliaPy/SymPy.jl/blob/afaca4a9e0951322dc907402388d1ef5651015b7/src/matrix.jl#L85. We could sort before returning, but you could also sort before using. If you want to put in a PR, it would be welcome.

mzaffalon commented 1 year ago

@jverzani Why sorting? Do eigvals and eigvecs not have the same ordering?

jverzani commented 1 year ago

I’m assuming that the eigenvals are sorted differently in sympy than Julia. If so, perhaps adjusting that would make things more in line. I’m not sure it would work with symbolic values though.

aosidnf commented 1 year ago

i tried other versions of sympy and python (the version in nixos 22.05, and the latest 1.11 in master) and had the same issue. I think sympy simply doesn't guarantee the ordering will be consistent. It seems sympy.ordered() will do the job, I'll make a PR later.