Deprecate TwoQubitGate and ThreeQubitGate

[kevinsung]

What is the use of gate_features.TwoQubitGate (and ThreeQubitGate, for that matter)? It doesn't define any method other than _num_qubits_.

The reason I think its existence could potentially be bad comes up when people want to count the number of two-qubit gates in their circuit. They might think to do

circuit.findall_operations_with_gate_type(cirq.TwoQubitGate)

but for this to work properly, every two-qubit gate in their circuit must be of a type explicitly declared to be a TwoQubitGate. I myself am wary of doing this and find myself always using the more sure-fire

circuit.findall_operations(lambda op: isinstance(op, cirq.GateOperation) and len(op.qubits) == 2)

instead.

[Strilanc]

It's a convenience class that avoids the user having to write a num_qubits method. It used to be more important, when gates weren't required to specify a qubit count and you had to write your own validation of the count.

I agree that it could cause the mistake you're pointing out. Same for the 1 and 3 qubit variants.

I am in favor of deprecating them.

[gecrooks]

Are gate_features like TwoQubitGate placed in the right place in the class definitions? For instance, XPowGate is declared as:

class XPowGate(eigen_gate.EigenGate, gate_features.SingleQubitGate):

But SingleQubitGate seems to be Mixin class, and EigenGate is the base class, so the order should be the other way around (Base class last)?

[bryano]

Can't the convenience can be provided by a decorator (à la value_equality) while addressing Kevin's concern?

[Strilanc]

Will tools recognize that the decorator is adding the abstract methods? We could end up creating a lot of false positives in pycharm.

I am in favor of deleting two qubit and three qubit gate, since it's so easy to just add a num qubits method now.

For single qubit gate, there's the issue of "on_each".

[bryano]

All the arguments here also apply to InterchangeableQubitsGate.

[Strilanc]

InterchangeableQubitsGate actually has a purpose. It affects the equality check in GateOperation.

That being said, we could certainly replace that class with an optional method that Gate classes can override.

[bryano]

It has a purpose in the same sense as the others: it defines a method (qubit_index_to_equivalence_group_key). The cost of missing some "interchangeable gates" by checking by type rather than by "equivalence group" is admittedly much smaller than undercounting two-qubit gates, but it seems like the same sort of issue.

[dabacon]

on_each could be generalized for SingeQubitGate, for a two qubit gate it would just apply to pairs in the list. For k-qubit gate it would just iterate in steps of size k.