Add cirq.merge_k_qubit_unitaries transformer to replace cirq.MergeSingleQubitGates optimizer
#4986
Conversation
…ubit_gates transformer
tanujkhattar
requested review from
a team,
vtomole,
cduck,
dabacon,
ColemanCollins,
nakardo,
gmauricio,
wcourtney and
verult
as code owners
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
| """ | ||
| if rewriter is not None and synthesizer is not None: | ||
| raise ValueError("Can't specify both rewriter and synthesizer.") | ||
| merged_circuit_op_tag = "_merged_single_qubit_gates_component" |
There was a problem hiding this comment.
Should this be a global ?
There was a problem hiding this comment.
No, the tag should be local because it's used as an implementation detail to identify circuit operations which were created by the internal merge and given them special treatment while passing arguments to rewriter. Note that this tag is never applied to any operation of the final circuit being returned from the function.
|
|
||
|
|
||
| @transformer_api.transformer | ||
| def merge_single_qubit_gates_to_phased_x_and_z( |
There was a problem hiding this comment.
Do we still need this transformer ?
There was a problem hiding this comment.
I think we should still keep this transformer as it's hard to predict all the user call sites where it might be getting used and whether phsx version would be an ideal replacement or not. There's also very little overhead to keep it
…e_single_qubit_gates
…e_single_qubit_gates
…e_single_qubit_gates
tanujkhattar
changed the title
cirq.merge_single_qubit_gates transformer to replace cirq.MergeSingleQubitGates optimizercirq.merge_k_qubit_unitaries transformer to replace cirq.MergeSingleQubitGates optimizer
|
@MichaelBroughton This is ready for re-review. PTAL |
| def merge_k_qubit_unitaries( | ||
| circuit: 'cirq.AbstractCircuit', | ||
| *, | ||
| context: Optional['cirq.TransformerContext'] = None, | ||
| k: int = 0, | ||
| rewriter: Optional[Callable[['cirq.CircuitOperation'], 'cirq.OP_TREE']] = None, | ||
| ) -> 'cirq.Circuit': |
There was a problem hiding this comment.
I'm a little confused now, this seems like a kind of drastic API change. What is k for if it can only be zero or one ? This also errors by default ?
There was a problem hiding this comment.
k can by any integer > 0 . This will be useful to replace MergeInteractions and MergeInteractionsToSqrtISwap, both of which need to merge connected components of 2q gates. We can directly call this method with k=2 instead of rolling out another implementation.
| @transformer_api.transformer | ||
| def merge_single_qubit_moments_to_phxz( | ||
| circuit: 'cirq.AbstractCircuit', | ||
| *, | ||
| context: Optional['cirq.TransformerContext'] = None, | ||
| atol: float = 1e-8, | ||
| ) -> 'cirq.Circuit': |
There was a problem hiding this comment.
Why do we need a moment based transformer in addition to a gate based one ?
There was a problem hiding this comment.
See the tests for an example use case.
The idea is that if I give you a moment preserving circuit with alternating layers and 1 and 2q gates that you need to convert to a target gateset, during conversion you will often end up in a situation where there are two adjacent moments of single qubit gates. Calling an operation based transformer to merge the single qubit gates is not sufficient as it can result in 2 separate moments in the final result, whereas a moment based transformer will guarantee that the two moments are merged into one.
…e_single_qubit_gates
…SingleQubitGates` optimizer (quantumlib#4986) * Replace cirq.MergeSingleQubitGates optimizer with cirq.merge_single_qubit_gates transformer * Add merge_k_qubit_unitaries primitive
…SingleQubitGates` optimizer (quantumlib#4986) * Replace cirq.MergeSingleQubitGates optimizer with cirq.merge_single_qubit_gates transformer * Add merge_k_qubit_unitaries primitive
…SingleQubitGates` optimizer (quantumlib#4986) * Replace cirq.MergeSingleQubitGates optimizer with cirq.merge_single_qubit_gates transformer * Add merge_k_qubit_unitaries primitive
cirq.merge_k_qubit_unitariescirq.merge_single_qubit_gates_to_phased_x_and_zcirq.merge_single_qubit_gates_to_phxzcirq.merge_single_qubit_moments_to_phxz