Add cirq.optimize_for_target_gateset transformer and cirq.CompilationTargetGateset interface
#5005
Conversation
There was a problem hiding this comment.
Few small nits and then a few high level questions. My biggest concern is if we should ship something in decompose_operations_to_target_gateset that looks so similar to decompose. My thinking is that less is more and it's probably best if we don't give users too many ways to do the same thing.
| def decompose_operations_to_target_gateset( | ||
| circuit: 'cirq.AbstractCircuit', | ||
| *, | ||
| context: Optional['cirq.TransformerContext'] = None, | ||
| gateset: Optional['cirq.Gateset'] = None, | ||
| decomposer: Callable[['cirq.Operation', int], DecomposeResult] = lambda *_: NotImplemented, | ||
| ignore_failures=True, | ||
| ) -> 'cirq.Circuit': |
There was a problem hiding this comment.
Do we need this transformer ? It seems like a user should be able to fairly easily accomplish this same thing by just calling into the regular decompose. Having this as a thin wrapper over decompose seems like it might be complicating things for the users.
There was a problem hiding this comment.
Replied in the main comment. Tl;Dr is that having benefits of making it a transformer, like automated logging + no-compile tags, make it worth it. Plus we have other similar wrappers like expand_composite.
| @property | ||
| @abc.abstractmethod | ||
| def num_qubits(self) -> int: | ||
| """Maximum number of qubits on which a gate from this gateset can act upon.""" |
There was a problem hiding this comment.
Can this be defined here as something like:
return max(g.num_qubits() for g in self.gates)?
There was a problem hiding this comment.
Gateset is composed of gate families, and gate families don't have a num_qubits parameter. For example, it's possible to create gate families which can accept any controlled single qubit rotation with one or more controls. A num_qubits parameter cannot be precisely defined in this case.
Enforcing num_qubits on gate families would be a much broader change without significant benefit outside the scope of compilation target gatesets.
| """Abstract base class to create gatesets that can be used as targets for compilation. | ||
|
|
||
| An instance of this type can be passed to transformers like `cirq.convert_to_target_gateset`, | ||
| which can transform any given circuit to contain gates accepted by this gateset. |
There was a problem hiding this comment.
This class seems important enough for users that it might be good to give a snippet of a simple implementation in the docstring. WDYT ?
There was a problem hiding this comment.
Concrete implementations will be added in the follow-up PR. Maybe we can then update the docstrings here to include a hyper-linked example to implementation? Something like
"Please see cirq.CZTargetGateset for a concrete implementation." ?
The primary benefit here is the automated logging + support for no-compile tags, that comes with making it a transformer. Given this benefit, and given that we already have other transformers like |
There was a problem hiding this comment.
LGTM. The closeness of decompose_operations_to_target_gateset to decompose still worries me a bit. Could we do something like:
decompose_operationst_to_target_gateset -> convert_to_gateset
convert_to_target_gateset -> fuse_and_convert_to_gateset
Just to avoid the overlap with decompose and in the case of fuse_and_convert_to_gateset communicate the fact that it won't be just a simple conversion. WDYT ?
|
As discussed offline,
|
tanujkhattar
changed the title
cirq.convert_to_target_gateset transformer and cirq.CompilationTargetGateset interfacecirq.optimize_for_target_gateset transformer and cirq.CompilationTargetGateset interface
…nTargetGateset` interface (quantumlib#5005) * Add convert_to_target_gateset transformer and CompilationTargetGateset interface * Override validation_operation to not accept intermediate results
…nTargetGateset` interface (quantumlib#5005) * Add convert_to_target_gateset transformer and CompilationTargetGateset interface * Override validation_operation to not accept intermediate results
…nTargetGateset` interface (quantumlib#5005) * Add convert_to_target_gateset transformer and CompilationTargetGateset interface * Override validation_operation to not accept intermediate results
cirq.CompilationTargetGateset: Interface for defining compilation targets._decompose_operations_to_target_gateset: Transformer to locally convert each operation to target gateset.cirq.optimize_for_target_gateset: Transformer to convert a circuit to a target gateset -- also supports running pre and post processing transformers before decomposing individual operations.cirq.convert_to_target_gatesetand implementations of CZ, SqrtIswap target gatesets. #5003 for larger context and implementations of specific target gatesets deriving fromcirq.CompilationTargetGateset.cc @MichaelBroughton @dstrain115