noncliff: add tests for conjugate destabs #413
Conversation
Fe-r-oz
force-pushed
the
fa/removeconjtodo
branch
from
c82ed55 to
2a5c4b8
Compare
|
|
||
| @testset "conjugate destabs" begin | ||
| @testset "Single qubit Clifford gate" begin | ||
| for s in [S"X", S"Y", S"Z", S"-X", S"-Y", S"-Z"] |
There was a problem hiding this comment.
to make the tests more trustworthy we should have multi-qubit cases and cases of non-clifford operations -- it can be in a separate testset to keep this already nice testset easy to read. The extra testset can contain stuff like:
for n in [5, 10, 100]
for g in [tHadamard, tPhase, pcT]
eg = embed(g,...)
sm = GeneralizedStabilizer(random_stabilizer(n))
...
There was a problem hiding this comment.
embed is not defined for CliffordOperator, only for PauliOperator, so instead used: enumerate_cliffords(i, clifford_cardinality(j)) for multi-qubit CliffordOperators. I hope you like it.
There was a problem hiding this comment.
Instead of embed for CliffordOperator, we can use CliffordOperator(sHadamard(2), 5)... darn...
Fe-r-oz
force-pushed
the
fa/removeconjtodo
branch
from
2a5c4b8 to
d983a48
Compare
Krastanov
added
the
Skip-Changelog
Fe-r-oz
force-pushed
the
fa/removeconjtodo
branch
from
5660561 to
157fcbb
Compare
|
Thank you for your speedy comments and insights. That is much appreciated! I hope this is satisfactory. |
| (1, [tHadamard, tPhase, tId1]), | ||
| (2, [tCNOT, tCPHASE, tSWAP]), | ||
| (3, [enumerate_cliffords(3, clifford_cardinality(3)), CliffordOperator(sHadamard(3), 3), CliffordOperator(sCNOT(1, 2), 3)]), | ||
| (4, [enumerate_cliffords(4, clifford_cardinality(4)), CliffordOperator(sHadamard(4), 4), CliffordOperator(sCNOT(2, 1), 4)]), | ||
| (5, [enumerate_cliffords(5, clifford_cardinality(5)), CliffordOperator(sHadamard(5), 5), CliffordOperator(sCNOT(2, 3), 5)]) |
There was a problem hiding this comment.
this seems unnecessarily complicated. And the number of qubits is something that does not need to be specified separately
in particular, is there something special about enumerate_cliffords(n, clifford_cardinality(n)) that you are picking this choice
I will submit a few simplification suggestions later today
There was a problem hiding this comment.
is there something special about enumerate_cliffords(n, clifford_cardinality(n)) that you are picking this choice
Nope. No particular reason for the choice of parameters for enumerate_cliffords.
Thank you!
There was a problem hiding this comment.
to keep in the spirit of having a random check you can also just do enumerate_clifford(3, rand(1:clifford_cardinality(3))), but at this point it is just simpler to stick to random_stabilizer
|
thanks for pushing through this, it is very helpful cleanup. Let's wait to see what the new tests say |
This PR removes the TODO comment and add tests for conjugation. This PR is inspired from this comment: #408 (comment)