Add two-qubit unitary decomposition for (inverse) sqrt-iSWAP #4213
Conversation
cduck
force-pushed
the
sqiswap
branch
2 times, most recently
from
8004fda
to
8fd5c4e
Compare
cduck
changed the title
|
|
|
PTAL @tanujkhattar @dstrain115 @vtomole. Thanks for all the comments. I removed the conditional gate cleanup code and always output nice XPow/YPow/ZPow gates but no global phase. Let me know if I missed updating any doc strings. |
|
Since no one re-reviewed this yet, I added the |
cduck
changed the title
|
Ping. |
|
As Tanuj is on reduced schedule this week, I can help out with the review. I'll get to this today. |
| # No error tolerance needed | ||
| ieq1 = y > np.pi / 8 | ||
| ieq2 = z < 0 | ||
| if ieq1: |
There was a problem hiding this comment.
IIUC, this is different (and better :)) from the reference implementation given in the paper in Algorithm-1. where they keep track of the intermediate single qubit rotations during canonicalization of the interaction coefficients. Instead of keeping track of the rotations, we simply recompute the kak and recurse for both single & two qubit case here.
I think it's worth adding a small description explaining the property of kak vectors that we can compose unitaries corresponding to (x1, y1, z1) and (x2, y2, z2) to form unitary corresponding to (x1 + x2, y1 + y2, z1 + z2), which is used here to break the problem into subtasks.
There was a problem hiding this comment.
I'll do that. I couldn't get this part of the algorithm from the paper to work so this is actually an improvement over the paper that supports decomposing any gate into 3-sqrt-iSWAP.
There was a problem hiding this comment.
To clarify your statement, this doesn't "recompute" kak, it just uses kak_canonicalize_vector to do the canonicalization automatically instead of manually like in Algorithm 1.
| *, | ||
| single_qubit_gate_types=(cirq.ZPowGate, cirq.XPowGate, cirq.YPowGate), | ||
| two_qubit_gate=cirq.SQRT_ISWAP, | ||
| atol=1e-6, |
There was a problem hiding this comment.
It would be nice if the default atol in cirq.allclose_up_to_global_phase is enough. Right now, the default atol returns false in many cases.
There was a problem hiding this comment.
Agreed. There is some source of numerical error that I couldn't track down. It doesn't help that some the Cirq linalg functions don't pass through atol to half of the functions they use internally (so default to 1e-8).
There was a problem hiding this comment.
Added a comment. It may be worth looking into the cause of this and also fixing various linalg functions that don't pass atol to their sub-functions.
|
Thanks for the reviews. I'll try to do these minor fixes tomorrow otherwise early next week. |
cduck
added
the
automerge
…lib#4213) - Add the method cirq.two_qubit_matrix_to_sqrt_iswap_operations() to optimally decompose with SQRT_ISWAP or SQRT_ISWAP_INV gates. - Add cirq.SQRT_ISWAP and cirq.SQRT_ISWAP_INV (aliases for cirq.ISWAP**0.5 and cirq.ISWAP**-0.5).
Follow up to quantumlib#4213. Fixes quantumlib#4083. (Also contains a one-line change to fix quantumlib#4225.) I'm open to name suggestions for `MergeInteractionsToSqrtIswap`.
Part of #4083.
cirq.two_qubit_matrix_to_sqrt_iswap_operations()to optimally decompose withSQRT_ISWAPorSQRT_ISWAP_INVgates.cirq.SQRT_ISWAPandcirq.SQRT_ISWAP_INV(aliases forcirq.ISWAP**0.5andcirq.ISWAP**-0.5).