Add internal evolve methods by Clifford gates#15813
Add internal evolve methods by Clifford gates#15813ShellyGarion wants to merge 10 commits intoQiskit:mainfrom
Conversation
ShellyGarion
added
mod: quantum info
|
One or more of the following people are relevant to this code:
|
ShellyGarion
changed the title
| op = Operator(gate) | ||
| pauli = Pauli(label) | ||
| target = op.dot(pauli).dot(op.adjoint()) | ||
| value = Operator(pauli._append_circuit(qc)) |
There was a problem hiding this comment.
Can we test this with public API instead of private methods? Can't we just use pauli.evolve(gate)? Same comment for the other occurrences of private methods 🙂
There was a problem hiding this comment.
Note that we already have test_evolve_clifford1 (line 432) and test_evolve_clifford2 (line 496) that check the evolve method for 1 and 2 clifford gates. These tests are dedicated to check the specific internal _append_circuit method.
There was a problem hiding this comment.
In 0b8e957 I enhanced test_evolve_clifford_circuit that now checks the evolve method of a random clifford circuit and not only of a random clifford.
There was a problem hiding this comment.
But evolve calls _append_circuit, doesn't it? I'm not sure I see the advantage of testing internal methods, which we generally avoid unless there's no other sufficient way of testing.
There was a problem hiding this comment.
The evolve method actually implements two internal algorithms. In case of the Clifford objects it calls _evolve_clifford (evolve by the clifford tableaux), and in case of a QuantumCircuit it calls _append_circuit
There was a problem hiding this comment.
My point is that we should test the public interface 🙂 Private methods are subject to arbitrary change so it could mean we have to update the tests -- whereas public interfaces remain the same and the tests ensure we remain backward compatible.
There's cases where it's difficult to properly test the functions with public interfaces only, but this is not the case here since it's fairly straightforward that Gates will actually go to _append_circuit 🙂
There was a problem hiding this comment.
As you can see from the tests, the check of the _append_circuit method is in addition to the more general evolve method. If you think it's redundant then I can remove it.
There was a problem hiding this comment.
Yeah I think it would be better to remove it (not because it is redundant but because it ties the test to unstable interfaces) - thanks! 🙂
4 participants
Summary
Add internal
evolvemethods by gate for some Clifford gates that were missing:_evolve_sx, _evolve_sxdg, _evolve_dcx, _evolve_iswap.Details and comments
see also: #15798 (comment)