Add cirq.CZTargetGateset to replace cirq.ConvertToCZAndSingleGates and cirq.MergeInteractions

[tanujkhattar]

• Adds the following new primitives:

  • cirq.TwoQubitCompilationTargetGateset: Abstract base class to facilitate defining two qubit compilation target gatesets.
  • cirq.CZTargetGateset: Compilation target gatesets for CZ + Single Qubit Rotations + Measurement gate. Used to deprecate cirq.MergeInteractions optimizer and cirq.ConvertToCZAndSingleGates optimizer.

• Part of Organization (and deprecation) of cirq-core/optimizers in cirq-core/transformers #4722

• Follows the new Transformer API Compiling: Circuit Transformers API #4483

• Supports no compile tags NoCompile Tag for optimizers NoCompile Tag for optimizers #4253

• A step towards fixing cirq.MergeInteractions should also allow FSim and other future decompositions #4214

cc @MichaelBroughton @dstrain115

[MichaelBroughton]

Few high level questions before detailed review. Main one is why do we need another level of interface granularity between TwoQubitCompilationTargetGateset and CompilationTargetGateset ? It seems the gains in having this are pretty small unless I'm missing something.

[MichaelBroughton]

Just curious: how did this happen ?

[tanujkhattar]

This happens because the newly introduced transformer is moment-preserving. Specifically, the original circuit is:

0: ───X───Y───Z───X^0.5───Y^0.5───S───X^-0.5───Y^-0.5───S^-1───H───@───X^0.25───Y^0.25───T───@───
                                                                   │                         │
1: ────────────────────────────────────────────────────────────────@───@─────────────────────@───
                                                                       │
2: ────────────────────────────────────────────────────────────────────@───────────────────────── 

In the first step, the following connected component gets merged and compiled:

      [ 0: ───X───Y───Z───X^0.5───Y^0.5───S───X^-0.5───Y^-0.5───S^-1───H───@───X^0.25───Y^0.25───T─── ]
0: ───[                                                                    │                          ]────────────────────────────────────────
      [ 1: ────────────────────────────────────────────────────────────────@───────────────────────── ]['_default_merged_k_qubit_unitaries']
      │
1: ───#2───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────

as a result we have the following (moment-preserving) circuit:

0: ───PhXZ(a=0.5,x=0.5,z=0)───@───PhXZ(a=0.304,x=0.333,z=0.142)───────@───
                              │                                       │
1: ───────────────────────────@───────────────────────────────────@───@───
                                                                  │
2: ───────────────────────────────────────────────────────────────@───────

whereas in the original workflow, the CZ(1, 2) would have overlapped with PhXZ in moment-2. The final compilation step beyond this point (ConvertToPauliStringPhasors) has not yet been migrated to the new primitives, as a result in the final output, CZ(1, 2) is just shifted to the right by 1 unit but still overlaps with a single qubit gate introduced due to ConvertToPauliStringPhasors.

This is technically a breaking change but I'm not too worried because it's in contrib/.

[tanujkhattar]

Main one is why do we need another level of interface granularity between TwoQubitCompilationTargetGateset and CompilationTargetGateset ? It seems the gains in having this are pretty small unless I'm missing something.

This is to avoid replicating the logic currently implemented in decompose_to_target_gateset in every implementation, which does the following:

1. Identify newly created connected components of 1q and 2q interactions (op_tree_old)
2. Find analytical decomposition of the component using _decompose_two_qubit_operation (op_tree_new).
3. Replace op_tree_old with op_tree_new iff:
   a. Either op_tree_old contains any 2q interaction which is not part of the current gateset
   b. OR all 2q interactions in op_tree_old are part of the current gateset but op_tree_new contains less number of 2q interactions (and hence is more efficient) than op_tree_old.

Note that 3b will not necessarily be the case always and therefore it's important to not replace op_tree_old with op_tree_new if op_tree_old is a more efficient implementation compared to op_tree_new. This logic currently also exists in both MergeInteractions and MergeInteractionsToSqrtISwap.

[MichaelBroughton]

Few nits, but after resolved we should be good to merge.

[MichaelBroughton]

If we expect users of this to interface to atleast have to override _decompose_two_qubit_operation we should probably make it public ?

[tanujkhattar]

I think we should keep it private because _decompose_two_qubit_operation is not supposed to be accessed from outside the class. The docstrings do specify that self._decompose_two_qubit_operation is an abstract method and should be overwritten by the users. I don't see why we need to make it public.

[tanujkhattar]

@MichaelBroughton Addressed all comments and also added serialization support for the cirq.CZTargetGateset class. PTAL.

[MichaelBroughton]

LGTM

[verult]

Is PhasedXZ general enough to multiple architectures to be placed in cirq-core?