Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pfaffian(A::SkewHermitian{<:Complex}) is missing #136

Closed
jlbosse opened this issue Apr 27, 2024 · 6 comments · Fixed by #137
Closed

pfaffian(A::SkewHermitian{<:Complex}) is missing #136

jlbosse opened this issue Apr 27, 2024 · 6 comments · Fixed by #137

Comments

@jlbosse
Copy link
Contributor

jlbosse commented Apr 27, 2024

Currently there is no support for computing pfaffians of complex matrices. TopologicalNumbers.jl has an implementation of the algorithms from Efficient numerical computation of the pfaffian for dense and banded skew-symmetric matrices but is a fairly heavy package.

If there is interest I can open a PR adding support for pfaffians of complex matrices.

@stevengj
Copy link
Member

stevengj commented Apr 27, 2024

The Pfaffian is defined for complex skew-antisymmetric matrices $A = -A^T$, but there is no conventional definition (as far as I can tell) for complex skew-Hermitian matrices $A = -A^*$.

This package doesn't have any type for complex skew-symmetric matrices $A = A^T$ right now. I suppose we could add one, but it's a fairly heavyweight change just to add one operation.

(We had a discussion offline about this with @alanedelman in 2022, and the conclusion was that there does not seem to be any useful natural definition of a Pfaffian for complex skew-Hermitian matrices.)

@stevengj
Copy link
Member

stevengj commented Apr 27, 2024

Actually, I suspect that the SkewLinearAlgebra._exactpfaffian! function probably works as-is for complex skew-symmetric matrices (though I haven't tested it). So, if we didn't want to add a new type, all that would be needed would be to add a method like:

pfaffian(A::AbstractMatrix{T}) where {T<:Complex} =
    pfaffian!(copyto!(similar(A, typeof(zero(T)/one(T))), A))
function pfaffian!(A::AbstractMatrix{<:Complex})
    LinearAlgebra.require_one_based_indexing(A)
    isskewsymmetric(A) || throw(ArgumentError("Pfaffian requires a skew-symmetric matrix"))
    return _exactpfaffian!(A)
end

along with a function isskewsymmetric(A).

This seems like it would be a useful addition if you want to add it along with tests and documentation.

@jlbosse
Copy link
Contributor Author

jlbosse commented Apr 27, 2024

Ah sorry, yes I did indeed mean complex skew-symmetric, not skew-hermitian. Regarding using _exactpfaffian! instead of e.g. one of the algorithms described in Efficient numerical computation of the pfaffian for dense and banded skew-symmetric matrices: Is exact_pfaffian numerically stable enough and/or fast enough?

@stevengj
Copy link
Member

stevengj commented Apr 27, 2024

As for speed, all algorithms for the Pfaffian of a dense $n \times n$ matrix are presumably $\Omega(n^3)$, and _exactpfaffian! is $\Theta(n^3)$ so it should be fine up to constant factors.

No idea about numerical stability for inexact arithmetic, but experimentally it seems fine:

julia> A = skewhermitian!(randn(100,100));

julia> p1, p2 = pfaffian(A), SkewLinearAlgebra._exactpfaffian!(copy(A.data))
(-7.982243401303055e30, -7.982243401308707e30)

julia> abs(p1 - p2) / abs(p1)
7.080738143649332e-13

julia> A = skewhermitian!(randn(300,300));

julia> p1, p2 = pfaffian(A), SkewLinearAlgebra._exactpfaffian!(copy(A.data))
(4.151962041555626e129, 4.151962041553932e129)

julia> abs(p1 - p2) / abs(p1)
4.0792054290158497e-13

though it does seem more susceptible to overflow than I would like:

julia> A = skewhermitian!(randn(500,500));

julia> p1, p2 = pfaffian(A), SkewLinearAlgebra._exactpfaffian!(copy(A.data))
(-1.5562908514995813e244, NaN)

but of course, once you get that big you will quickly overflow any algorithm and will have to switch to a logpfaffian method or similar.

@stevengj
Copy link
Member

It would be great to benchmark it (for both performance and accuracy) against the algorithm in TopologicalNumbers.jl, though.

@smataigne
Copy link
Collaborator

Thank you @jlbosse for your remarks! And thank you @stevengj for your interesting answers!

smataigne pushed a commit that referenced this issue Apr 28, 2024
So far I added only computation of pfaffians for complex skew symmetric
matrices, but if we want to take them seriously we should probably also
add a type `SkewSymmetric` and use that for pfaffian computations
instead of `SkewSymmetric` since that is what pfaffians are actually
defined for. However, this is out of the scope of this PR

This fixes #136
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants