XanaduAI / thewalrus

A library for the calculation of hafnians, Hermite polynomials and Gaussian boson sampling.
https://the-walrus.readthedocs.io
Apache License 2.0
101 stars 55 forks source link

Add power trace optimization conforming to c++14 #199

Closed trevor-vincent closed 3 years ago

trevor-vincent commented 3 years ago

Context:

The power trace computation in eigenvalue_hafnian uses Eigen's diagonalization routine. This can be made potentially faster by computing the characteristic polynomial first. Eigen also doesn't support long double.

Description of the Change:

Use the Labudde algorithm and optimization in Appendix B of Nico's paper to compute the power trace faster than Eigen for most sizes. Labudde is faster and more stable than Fadeev-Leverrier, see more here: https://arxiv.org/abs/1104.3769. With this change it will be easier to optimize the eigenvalue hafnian algorithm even further if needed.

Benefits:

Speed-up Removes Eigen from the hafnian calculation Long double support

Possible Drawbacks:

Related GitHub Issues: Fadeev-Leverrier algorithm for power traces #36

trevor-vincent commented 3 years ago

Sorry for the large delay on this, I went down the tensor network rabbit hole and didn't come out. Won't happen again! Everything is now conforming to C++11. I have improved the code a bit more as well. I have taken into account all of Josh's/Nico's suggestions from the previous pull request (which I closed because it's been so long... might as well start again).

nquesada commented 3 years ago

It seems like there are a lot of diffs that are just formatting. @josh146 can correct me if I am wrong, but It would be nice to pass astyle on the code http://astyle.sourceforge.net/ (or whichever other formatter we have used in the past) so that the diff is reduced and is easier to parse what the changes are.

josh146 commented 3 years ago

Yep, astyle is the one we have been using!

trevor-vincent commented 3 years ago

Yes good catch, I will fix that!

nquesada commented 3 years ago

Funnily enough, after this the eigenvalue algorithm will not be using eigenvalues.

codecov[bot] commented 3 years ago

Codecov Report

Merging #199 into master will not change coverage. The diff coverage is n/a.

@@            Coverage Diff            @@
##            master      #199   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           20        20           
  Lines         1131      1131           
=========================================
  Hits          1131      1131           

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 658e62a...e0faf1a. Read the comment docs.