-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enable pulling through Clifford operations, also add an option of only applying dd to single qubit gate moments #6675
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6675 +/- ##
========================================
Coverage 97.83% 97.83%
========================================
Files 1075 1077 +2
Lines 92325 92493 +168
========================================
+ Hits 90324 90492 +168
Misses 2001 2001 ☔ View full report in Codecov by Sentry. |
72ddf41
to
9368cd4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the following logic would be useful to test the correctness of the final circuits:
sampler = cirq.Simulator(dtype=np.complex128)
psi0 = sampler.simulate(circuit).final_state_vector
psi1 = sampler.simulate(transformed_circuit).final_state_vector
assert np.isclose(np.abs(np.vdot(psi0, psi1))**2, 1.0)
In particular, if this is the original circuit:
0: ───────────────────────────────────H───@───H───H───
│
1: ───────────────────────────H───@───H───@───────H───
│
2: ───────────────────H───@───H───@───────────────H───
│
3: ───────────H───@───H───@───────────────────────H───
│
4: ───H───@───────@───────────────────────────────H───
│
5: ───H───@───H───@───────────────────────────────H───
│
6: ───────────H───@───H───@───────────────────────H───
│
7: ───────────────────H───@───H───@───────────────H───
│
8: ───────────────────────────H───@───H───@───────H───
│
9: ───────────────────────────────────H───@───H───H───
Then with schema='X_XINV'
and single_qubit_gate_moments_only=True
, it gets transformed to
0: ───────────────────────────────────H───@───H───H────────────────────────
│
1: ───────────────────────────H───@───H───@───X───PhXZ(a=-0.5,x=0.5,z=0)───
│
2: ───────────────────H───@───H───@───X───────X───H────────────────────────
│
3: ───────────H───@───H───@───X───────X───────X───PhXZ(a=-0.5,x=0.5,z=0)───
│
4: ───H───@───X───@───X───────X───────X───────X───PhXZ(a=-0.5,x=0.5,z=0)───
│
5: ───H───@───H───@───X───────X───────X───────X───H────────────────────────
│
6: ───────────H───@───H───@───X───────X───────X───PhXZ(a=-0.5,x=0.5,z=0)───
│
7: ───────────────────H───@───H───@───X───────X───H────────────────────────
│
8: ───────────────────────────H───@───H───@───X───PhXZ(a=-0.5,x=0.5,z=0)───
│
9: ───────────────────────────────────H───@───H───H────────────────────────
which fails this test.
If I try the same transformation with single_qubit_gate_moments_only=False
, then I get the following circuit
0: ───────X───X───X───X───X───X───────H───@───H───H───
│
1: ───────X───X───X───X───────H───@───H───@───────H───
│
2: ───────X───X───────H───@───H───@───X───X───────H───
│
3: ───────────H───@───H───@───X───X───X───X───────H───
│
4: ───H───@───────@───X───X───X───X───X───X───────H───
│
5: ───H───@───H───@───X───X───X───X───X───X───────H───
│
6: ───────────H───@───H───@───X───X───X───X───────H───
│
7: ───────X───X───────H───@───H───@───X───X───────H───
│
8: ───────X───X───X───X───────H───@───H───@───────H───
│
9: ───────X───X───X───X───X───X───────H───@───H───H───
which doesn't quite behave as expected; many qubits could fit two more cirq.X
gates. (Also the more ideal behavior would be to not add any DD gates prior to the first moment that acts on that qubit.)
I suspect that the bug is in the logic of how to pull Pauli gates through Clifford gates (e.g. |
and
both passed for all test cases.
where ? can be any single-qubit-gate, e.g.,
Is this expected? I suspect we can't commute through the moment |
c462170
to
a80c9b7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!! (with one small nit)
Thank you so much, Renyi!
|
||
|
||
@transformer_api.transformer | ||
def add_dynamical_decoupling( | ||
circuit: 'cirq.AbstractCircuit', | ||
*, | ||
context: Optional['cirq.TransformerContext'] = None, | ||
schema: Union[str, Sequence['cirq.Gate']] = 'X_XINV', | ||
schema: Union[str, Tuple['cirq.Gate', ...]] = 'X_XINV', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's change the default schema to (cirq.X, cirq.Y, cirq.X, cirq.Y)
. I think that gives better performance experimentally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed that if the circuit ends in a measurement, e.g.
0: ───────────H───@───H───M───
│ │
1: ───H───@───────@───────M───
│ │
2: ───H───@───H───@───────M───
│ │
3: ───────────H───@───H───M───
then the transformer misbehaves. In this case, it outputs
0: ───────────H───@───H───M───X───
│ │
1: ───H───@───X───@───X───M───────
│ │
2: ───H───@───H───@───X───M───X───
│ │
3: ───────────H───@───H───M───────
One option is to raise a NotImplementedError
if the circuit contains measure gates. Alternatively, you could add support for circuits that include measure gates. (You can't pull single-qubit gates through measure gates, and no need to add DD unless there are more gates after the measure gate.)
Also enabling only apply dd to single qubit gates moments. Multiple test cases with diagrams in docstrings are added.
a80c9b7
to
7aad4e9
Compare
Now, we rely on |
Also changed default schema in the new commit. |
1a074c6
to
5fa81d5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thank you, Renyi!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there a way to simplify this code?
The single qubit clifford algebra is implemented in the SingleQubitCliffordGate class this may help simplify the implementation
) | ||
|
||
|
||
def test_exceptions(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand this test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, should delete this.
84faa14
to
2a7f563
Compare
2a7f563
to
74305a0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
merging now to unblock users but this code can probably be simplified
…y applying dd to single qubit gate moments (quantumlib#6675)
Enable adding dynamical decoupling in single qubit gate moments only.
Adding mechanism is described in the code. In short:
2 test cases added for single qubit gates moments insertion mode:
Test case 1: add dd preserves the circuit.
Test case 2:
input_circuit's diagram is:
output diagram is