Add ApproximateTwoQubitTargetGateset. #5055
Conversation
|
Synced offline, going to close for now and convert to transformer to form a baseline that other CompilationTargetGatesets might leverage. I'll leave it to you @tanujkhattar to leave any broad strokes pointers in the code and then close this when you're ready. |
There was a problem hiding this comment.
My main comment is that we should abstract out this functionality of creating a gate tabulation and using it for decomposing an arbitrary matrix into base gate + single qubit gates in a separate class / transformer so we reuse it in different target gatesets / transformers.
| if not protocols.has_unitary(op): | ||
| return NotImplemented | ||
|
|
||
| if protocols.has_kraus(op): |
There was a problem hiding this comment.
If op has a unitary, then it should always have a kraus as well. This condition can be removed.
| q0, q1 = op.qubits | ||
| decomp = self._tabulation.compile_two_qubit_gate(protocols.unitary(op)) | ||
| ret = [] | ||
| for i in range(len(decomp.local_unitaries) - 1): | ||
| mats = decomp.local_unitaries[i] | ||
| for mat, q in zip(mats, [q0, q1]): | ||
| phxz_gate = single_qubit_decompositions.single_qubit_matrix_to_phxz(mat) | ||
| if phxz_gate is not None: | ||
| ret.append(phxz_gate(q)) | ||
| ret.append(self._base_gate(q0, q1)) | ||
|
|
||
| mats = decomp.local_unitaries[-1] | ||
| for mat, q in zip(mats, [q0, q1]): | ||
| phxz_gate = single_qubit_decompositions.single_qubit_matrix_to_phxz(mat) | ||
| if phxz_gate is not None: | ||
| ret.append(phxz_gate(q)) |
There was a problem hiding this comment.
I think the right level of abstraction for this logic of
- Creating a tabulation in the
__init__function and - Using the created tabulation for decomposing any given two qubit operation
opinto gates from the target gateset
should reside as a standalone class in the cirq-core/cirq/transformers/heuristic_decompositions/two_qubit_gate_tabulation.py OR as a standalone transformer in cirq-core/cirq/transformers, decorated with @cirq.transformer (which is also requested in #4059)
The main reason behind this proposal is that we'd like to reuse this logic at different places, like individual transformers and different target gatesets that wish to "fall-back" on the approximate decomposition the given operation to be decomposed is not part of a known set. (eg: the SycamoreTargetGateset introduced in #5054).
Seemed like a useful one to have. Might be a fix for #4362 ?
I tried the implementation based on NuOps mentioned in the issue where you try to gradient descent each individual decomposition single qubit gate parameters to boost fidelity and it was pretty slow.
This way seems to be slow on init but then fast on decomposition (more scalable). What do people think ?
@tanujkhattar The implementation isn't as clean as it could be and the tests aren't super exhaustive, let me know if you think this is on the right track and I can add more and do cleanup.