Skip to content
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

Can we use CircuitOperations to replace composite gates like QFT, SwapNetwork etc. ? #4338

Closed
tanujkhattar opened this issue Jul 20, 2021 · 5 comments
Labels
area/decompose area/gates kind/design-issue A conversation around design needs agreed design We want to do this, but it needs an agreed upon design before implementation triage/accepted there is consensus amongst maintainers that this is a real bug or a reasonable feature to add

Comments

@tanujkhattar
Copy link
Collaborator

tanujkhattar commented Jul 20, 2021

Gate Decomposition relationships are confusing (#930) and needs to be organized (#3242). The following diagram, generated via Visualizing Gate Relationships Colab, describes (best-effort) the current decompose relationships between gates.

image

One of the primary use cases of cirq.decompose protocol is to define composite gates like QuantumFourierTransformGate, SwapNetworkGate etc. Such composite gates / operations fit better under the category of useful CircuitOperations rather than fundamental basic gates.

This issue is to discuss whether we can replace these gates with CircuitOperations and how should this be done?

cc @95-martin-orion

@tanujkhattar tanujkhattar added kind/design-issue A conversation around design triage/discuss Needs decision / discussion, bring these up during Cirq Cynque area/decompose area/gates labels Jul 20, 2021
@95-martin-orion
Copy link
Collaborator

I wholeheartedly support this change. CircuitOperation provides a reliable structure for defining composite gates, and once #4336 is submitted it will have a much lower serialization overhead than existing composite gates. In addition, redefining these operations as CircuitOperations may allow us to refine the list of Operation subtypes, which would help with issues like #2536 (CircuitOperations become easier to identify if anything that isn't a GateOperation is a CircuitOperation).

IMO, the correct way to implement this is to convert existing composite gates to implementations of CircuitOperation: for example, a SwapNetworkGate might take a swap-ordering for a set of qubits in its constructor to generate a frozen circuit that performs that swap, then pass that circuit to the CircuitOperation super constructor.

@vtomole vtomole added needs agreed design We want to do this, but it needs an agreed upon design before implementation triage/accepted there is consensus amongst maintainers that this is a real bug or a reasonable feature to add and removed triage/discuss Needs decision / discussion, bring these up during Cirq Cynque labels Jul 28, 2021
@tonybruguier
Copy link
Contributor

#4309 would be affected too. I have no strong opinion on the proposal but will follow whatever is decided. Just adding the note for context.

@tanujkhattar
Copy link
Collaborator Author

Discussion on Cirq Sync:

  • We need to deprecate the objects gratefully and rewrite tutorials to encourage our users to use CircuitOperations. We should also formalize whether we should create new CircuitOperations by subclassing or have helper functions return new CircuitOperations.
  • Also, it'll become unclear which operations are composite in future so we need to formulate.
  • Optimizations across CircuitOperations boundaries are currently not supported and should also be handled carefully.

@daxfohl
Copy link
Contributor

daxfohl commented Aug 25, 2021

Playing devil's advocate here, but for common patterns like QFT, it may be that some devices have specific implementations of those things optimized for those devices. If we convert them to generic subcircuits, then we lose the ability to allow these optimizations. We can hack them back in with tags or something, but that seems like a design flaw.

@tanujkhattar
Copy link
Collaborator Author

As discussed in
https://tinyurl.com/cirq-organize-decompose-rfc#heading=h.jaeoqsjs6i9x, CircuitOperations should not be used to replace composite gates.

The drawbacks of each of the two possible ways for using circuit operations to replace composite gates are as follows:

Way-1: Factory method returning circuit op for composite gates.

Bad because it’s hard to determine whether a given circuit operation represents a QFT / Boolean hamiltonian gate etc.

Way-2: Subclass inheriting from circuit operation.

Comes with too much unnecessary baggage from circuit operation (eg: replace, repeat etc.)

cc @95-martin-orion I'm closing this now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/decompose area/gates kind/design-issue A conversation around design needs agreed design We want to do this, but it needs an agreed upon design before implementation triage/accepted there is consensus amongst maintainers that this is a real bug or a reasonable feature to add
Projects
None yet
Development

No branches or pull requests

5 participants