-
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
Dynamical decoupling: Support pulling through the whole circuit with non-clifford moments #6718
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6718 +/- ##
==========================================
- Coverage 97.83% 97.83% -0.01%
==========================================
Files 1077 1077
Lines 92537 92558 +21
==========================================
+ Hits 90535 90555 +20
- Misses 2002 2003 +1 ☔ View full report in Codecov by Sentry. |
a1eb1ac
to
f571ccf
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.
Looks good to me! Thank you so much, 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.
my original concern still holds, the DD logic is getting more complex. can you simplify it?
for qid, gate in enumerate(pulled_through_pauli_ops): | ||
pulled_through *= gate.on(qubit_order[qid]) | ||
if _is_clifford_op(op_at_moment): | ||
prev_circuit = cirq.Circuit(cirq.Moment(op_at_moment)) |
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.
this is inefficient. looks like you only need the unitary which you can get using cirq.protocols.unitary_protocol.unitary(op)
Previously, we only insert dd for split Clifford pieces. While, one use case is to insert into consecutive idle moments cross Clifford pieces. We also support merging remaining Puali op of dd sequence into starting / ending single-qubit operations of each consecutive idle ops.
08258a5
to
795bc43
Compare
I have simplified the code with some abstractions. Also added comments in a couple of places, especially pulling through mechanisms. |
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.
a few nits, otherwise LGTM
return False | ||
return True | ||
def _is_clifford_op(op: 'cirq.Operation') -> bool: | ||
if op.gate is not None and isinstance(op.gate, cirq.MeasurementGate): |
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.
if op.gate is not None and isinstance(op.gate, cirq.MeasurementGate): | |
if op.gate is not None or isinstance(op.gate, cirq.MeasurementGate): |
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 just noticed you import cirq. this will cause circular imports. please follow the google style for imports
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.
thanks for the changes, great job
Thanks for the careful review Nour, will submit the code shortly. |
Thank you both!! |
…non-clifford moments (quantumlib#6718) * Support a new feature of adding dd. Previously, we only insert dd for split Clifford pieces. While, one use case is to insert into consecutive idle moments cross Clifford pieces. We also support merging remaining Puali op of dd sequence into starting / ending single-qubit operations of each consecutive idle ops. * Pull through all the way. * Apply comments; simply the code a bit; Add comments. * apply reviews * import cirq -> import ops, ...
Previously, we only insert dd for split Clifford pieces. While, some use cases require
Example: schema=(X, Y, X, Y)