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

Add support for CCO's to Gatesets. #5004

Open
tanujkhattar opened this issue Feb 18, 2022 · 4 comments
Open

Add support for CCO's to Gatesets. #5004

tanujkhattar opened this issue Feb 18, 2022 · 4 comments
Labels
area/classical area/gatesets area/transformers kind/feature-request Describes new functionality priority/p2 Next release should contain it status/needs-agreed-design We want to do this, but it needs an agreed upon design before implementation triage/accepted A consensus emerged that this bug report, feature request, or other action should be worked on

Comments

@tanujkhattar
Copy link
Collaborator

Is your feature request related to a use case or problem? Please describe.
Gatesets currently accept any cirq.Gate instance or sub-type as a valid input parameter. For special operations, like GlobalPhaseOperation, CircuitOperation etc, they have flags like unroll_circuit_op or accept_global_phase_op to control the behavior.

ClassicallyControlledOperation was recently introduced as a new type of operation that wraps traditional gate operations but don't have an op.gate property. As a result, there's currently no way to specify in a gateset definition that a CCO should be accepted or not.

Describe the solution you'd like
Support for CCOs should be added to gatesets, either by adding an additional flag or by adding a .gate property to the CCO operation, which can simply return the .gate of it's underlying sub_operation.

What is the urgency from your perspective for this issue? Is it blocking important work?
P1 - I need this no later than the next release (end of quarter)

@tanujkhattar tanujkhattar added kind/feature-request Describes new functionality area/classical area/gatesets area/transformers priority/p2 Next release should contain it triage/discuss Needs decision / discussion, bring these up during Cirq Cynque labels Feb 18, 2022
@daxfohl
Copy link
Collaborator

daxfohl commented Feb 18, 2022

xref #4683

Also we generally support op.gate.on(q) is at least the same behavior as the original op, at least when gate exists. Tags are the only thing I can think of that doesn't carry over. So this would be a departure.

If we want to make CCO.gate return some CCGate class, this generally requires subcircuit-gates to be implemented too. #4725 (comment)

@vtomole vtomole added triage/accepted A consensus emerged that this bug report, feature request, or other action should be worked on status/needs-agreed-design We want to do this, but it needs an agreed upon design before implementation labels Feb 23, 2022
@vtomole
Copy link
Collaborator

vtomole commented Feb 23, 2022

This should be supported after we've agreed on a design.

@vtomole vtomole removed the triage/discuss Needs decision / discussion, bring these up during Cirq Cynque label Feb 23, 2022
@tanujkhattar
Copy link
Collaborator Author

@95-martin-orion To confirm, this is post 1.0 IMO. Please confirm.

@95-martin-orion
Copy link
Collaborator

@95-martin-orion To confirm, this is post 1.0 IMO. Please confirm.

Confirmed. This requires design, and CCOs remain functional without it in the meantime.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/classical area/gatesets area/transformers kind/feature-request Describes new functionality priority/p2 Next release should contain it status/needs-agreed-design We want to do this, but it needs an agreed upon design before implementation triage/accepted A consensus emerged that this bug report, feature request, or other action should be worked on
Projects
Status: No status
Development

No branches or pull requests

6 participants