Feature fpr algorithm performance #274
Conversation
Switch numpy calls to use faster eigenvalue calculation for symmetric matrices.
This adds a check for what was previously an edge case, but now is a common occurrence when applying this to FPR, where the update doesn't change the rank of the base matrix because that matrix is full rank already. In this case we fall back to the standard Woodbury inverse identity.
Now greedy search algorithm for per-germ FPR using a new objective function and low-rank update based performance enhancements.
The seed wasn't be plumbed in properly for the new greedy FPR algorithm.
Fixes a seeding issue in candidate circuit selection wherein the wrong seed was being passed into a function. Also modifies default behavior when the main seed is specified but a separate seed for the candidates is not.
I was getting some strange behavior when using complex dtypes with matrices we're assuming to be real. For now enforce a requirement that you use a real-valued data type when assume real is true and vice versa.
Speed up the construction of the compact EVD cache by using a connection between the left and right singular vectors.
Removes a bunch of unneeded debugging code and cleans up the logging.
Remove a bunch of unneeded debugging lines and print statements and clean up logging as needed.
The exit criterion used in the greedy search algorithm didn't take into account the rank of the candidate set's jacobian and so it was possible to have a satisfactory psuedoinverse trace while not being sufficiently high rank. This adds an explicit check for this.
Adds more logging to the randomized search indicating the maximum rank seen at each iteration.
|
I've spent some time looking into @enielse's concerns about the size of the fiducial sets returned for the randomized fiducial pair search routine. I've more or less convinced myself that this isn't a bug. I added additional logging to the randomized search routine so that the maximum rank seen for any Jacobian at each iteration (iteration = candidate fiducial set size) was printed. It looks like (for the particular test case using XYCNOT) that selecting fiducials at random just does a pretty poor job at giving us a full-rank Jacobian (this wouldn't have been my intuition, but it looks to be the case here). The objective function here can't be satisfied until the Jacobian is full-rank and so this is the limiting factor. Looking into this did help me identify a mistake in the greedy algorithm, by the way. So this was all worthwhile. I had mistakenly failed to include in the exit criterion a check to ensure that the Jacobian was full rank and, unlike the minimum eigenvalue criterion, the psuedoinverse trace condition can be satisfied without being full rank because of how the psuedoinverse is defined. That is fixed now. We now get fiducial sets that are a bit larger, but the greedy search really does seem to be outperforming the random search by a significant amount here. The greedy score function does favor updates that increase the rank of the fiducial set the most up until the point where the Jacobian becomes full-rank, so maybe this isn't too surprising. |
|
I did some digging too - and agree with @coreyostrove: I don't think there's necessarily a bug in the current FPR algorithm. The large number of pairs the old method is finding differs from earlier pyGSTi versions but from what I can tell this is due to bug fixes rather than introductions. Down the line, we should be able to trim down the list of needed pairs by tracking the parameter directions a germ was intended to amplify, but that's for another PR. |
pygsti/algorithms/germselection.py
Outdated
| print('central_mat shape: ', central_mat.shape) | ||
| #now calculate the diagonal elements of pinv_E_beta@central_mat@pinv_E_beta.T | ||
|
|
||
| #Take the cholesky decomposition of the central matrix: |
There was a problem hiding this comment.
Maybe add a comment here explaining the different execution paths. If the Cholesky decomp fails, it seems that you can fall back on just an eisum (line 3144) that I assume is slower than the route using the Cholesky decomp, is that right? If this is correct, I think just noting in a comment that the einsum on 3144 is what we're trying to accomplish and this can be optionally sped up via a Cholesky decomposition execution path (if it works).
There was a problem hiding this comment.
Thanks for the feedback, @enielse. I added some comments on this in my latest commit.
There was a problem hiding this comment.
All looks good to me! @coreyostrove, ball is in your court for whether you want to add the comments Erik is suggesting, or just merge this in now.
Adds comment detailing the theory used for the low-rank update version of the greedy search algorithm. Also renames the greedy search algorithms function name more appropriately.
Fixes a broken unit test by giving it a slightly larger memory limit (I must've messed with the memory limit calculation at some point).
|
I finally forced myself to sit down and write up a description of the theory behind the low-rank update code. I eventually plan to convert this into an expanded and more detailed write-up in latex (hence why it reads the way it does), so let me know if anything is unclear or confusing and I'll try to clarify that once I get around to writing that document. Edit: Also I fixed that germ selection unit test in algorithmsb I accidentally broke. Just needed a slightly larger memory limit (I vaguely recall changing how the memory estimate was calculated, so that must've been why). |
This branch introduces a new greedy search algorithm for performing per-germ FPR. This algorithm leverages the same low-rank update machinery used to generate speedups for germ selection. Due to the nature of the low-rank updates used, this utilizes an objective function based on the trace of the psuedo-inverse of a candidate fiducial set's Jacobian instead of the minimum eigenvalue (these should largely track with each other though). In the course of developing this a number of edge cases were revealed in the low-rank update code which didn't arise when testing germ selection. These have been patched, and as a nice side-effect the fixes actually result in a modest performance boost for germ selection too.
Smaller changes included in the branch include: