Refactor Gate class

[Mayank447]

@BoxiLi I think we should iterate over our ideas for refactoring the Gate class (#263) in the Discussion tab for any contributor to follow them in the future. Since they will be spread between multiple Issues and PRs otherwise.

[Mayank447]

Since #307 has been merged. We should focus on making Gate an abstract class (ABC) now. Since we are making Gate an abstract class, we need to set a good template for people defining own Primitive Gates or us improving the current standard gates.

class Gate(ABC):
    def __init__(self, name, num_qubits, label):
        if not validate_parameters(...):
             raise ValueError()
        ...

   @abstractmethod
   def validate_parameters(self) -> bool:
       ...

    @property
    def qubit_count(self) -> int:
        ...

    @property
    def is_clifford(self) -> bool:
        ...

    @property
    def self_inverse(self) -> bool:
        ...


    @abstractmethod
    def get_compact_qobj(self, num_qubits) -> Qobj:
        ...

    @abstractmethod
    def inverse_gate(self, num_qubits) -> Gate:
        ...

    def __str__(self) -> str:
        ...

    def __repr__(self) -> str:
        ...

    def __repr_latex(self) -> str:
        ...

This is what I have in mind. It assumes we are not storing targets, control, etc.

[Mayank447]

I remember some issues where you wanted to store GateDuration and gate_decomposition within the Gate class. If yes, what method definition for those would you proceed with.

[BoxiLi]

I think the template you wrote looks pretty good. Yes, I wanted a decomposition methods to be defined for each gate in the gate class, but that is a complicated API, since one gate can be decomposed into different basis gates (CNOT->iSWAP, CNOT->SWAP etc). I would suggest to add this later.

I do remember at some point there was a problem with duration. Don't really recall where. But this duration is a processor specific value, that is, the same gate may have different durations on different processor. So it should not be included at the unitary gate level here.

[BoxiLi]

I used get_compact_qobj to distinguish it with get_qobj, which gives one the full matrix Qobj including target qubit information. Now we separated the targets. I was wondering if this can be simplified to just get_qobj. If qutip-qip deprecate gate.py in the furture (I'm not sure how many people are still using them, may still quite some), people may what to get the Qobj representation of Toffoli by some short script like Toffoli.get_qobj ?

[Mayank447]

Yes, we can replace get_compact_qobj with get_qobj towards the tend. Also for non-parametric gates get_qobj will be a @staticmethod.

[Mayank447]

I used get_compact_qobj to distinguish it with get_qobj, which gives one the full matrix Qobj including target qubit information. Now we separated the targets. I was wondering if this can be simplified to just get_qobj. If qutip-qip deprecate gate.py in the furture (I'm not sure how many people are still using them, may still quite some), people may what to get the Qobj representation of Toffoli by some short script like Toffoli.get_qobj ?

After #319 get_qobj() isn't being used anymore in the codebase. Would you like me to rename get_compact_qobj() to get_qobj()?

Also regarding gates in gates.py (which should be depreciated), they are already defined in qutip.core (https://github.com/qutip/qutip/blob/master/qutip/core/gates.py#L12). I believe these were previously defined in qutip.qip before v5 and a copy was kept during migration (otherwise it would be surprising to have a molmer_sorensen gate which is only used in my experience in Trapped Ion.

[BoxiLi]

Yes let's rename get_compact_qobj() to get_qobj().

About the gates.py in qutip. Yes, we want to keep those in core qutip as there are many people using them. Much more than people who use qutip-qip. Actually in qutip-qip there are a few more gates defined, introduced after the separation. But never mind, I think we can deprecated (not break) gates.py in qutip-qip and point people either to the copy in qutip or the gate class. What do you think?

[Mayank447]

Sounds good

[Mayank447]

With #309 merged. We now need to focus on removing targets, controls from the Gate class. One major thing we need to handle before that is arg_value and arg_label. targets, controls must be stored in the QubitCircuit class instead of Gate class. But arg_value and arg_label should ideally be stored in the Gate class itself.

I believe we should have the parametric gates as Gate factory (a function which takes in the parameters and returns a Gate class instead of an object instance).

def RX(arg_value, arg_label) -> Gate:
    # Parameter verification logic, raises error otherwise

    class _RX(Gate):
        arg_value =  arg_value
        arg_label = arg_label

        @staticmethod
        def get_compact_qobj() -> Qobj:
               return ....
     
     return _RX

Once targets, controls are moved to QubitCircuit we should ideally make all the gate classes @dataclasses since there isn't anything dynamic about them.

From the API perspective, qc.add_gate("RX", targets=[0], arg_value=np.pi/2) would still work as will be handling it internally.

@BoxiLi What's your opinion on this

[BoxiLi]

I am not so familiar with this type of design. What will be the user interface if someone wants to define an RX gate withan angle pi? In many applications, it is common to repeatedly assign different angle values to the same Gate in a circuit. For instance, the Variational Quantum Algorithm (VQA). I feel like this will make the logic of parameter update very cumbersome?

[Mayank447]

Well the user interface would ideally be
RX(arg_value=np.pi) to define an RX gate with angle pi. You may refer to https://stackoverflow.com/questions/2526879/what-exactly-is-a-class-factory on designs based on class factory, they are actually quite common.

But throwing out a class for different parameters like in VQA would be inefficient and cumbersome. I will just leave Parameterized Gate it to be a standard class like how it is now and only remove the targets, classical controls.

Also what's your opinion on making non-parametrized gates as @dataclass since we won't need __init__ for any standard non-parametrized gate like X and way to define custom gates is via inheritance now.

[BoxiLi]

Well the user interface would ideally be RX(arg_value=np.pi) to define an RX gate with angle pi.

That is nice. Indeed, by itself, it does look quite elegant.

Also what's your opinion on making non-parametrized gates as @DataClass since we won't need init for any standard non-parametrized gate like X and way to define custom gates is via inheritance now.

I think it is good. It will make the code cleaner for us, right? For users who want to define a custom gate. They can inherit from the meta class and then do whatever they want I assume.

[Mayank447]

Yep, removing __init__ for non-parametrized gates will make the code cleaner.

[Mayank447]

With #316 merged. Let's focus on writing an abstract baseclass for CircuitInstruction, circuit.instructions will be a list of CircuitInstruction class object. On top of the base class we define GateInstruction, MeasurementInstruction, if someone wants, they can implement Barrier, reset similarly.

@BoxiLi This is the class design I have in mind

@dataclass(frozen=true)
class CircuitInstruction(ABC):
    operation: Gate | Measurement
    qubits: tuple[int] = tuple()
    cbits: tuple[int] = tuple()
    style: dict | None = None  # style is used by circuit draw

    def __post_init__(self):
        """Basic validation for all instructions."""
        if not len(self.qubits) and not len(self.cbits):
            raise ValueError("Instruction must operate on at least one qubit or cbit.")

       # Similar check each entry is an int and >=0

    @abstractmethod
    def __repr__(self):
        pass

These classes are frozen so that they are hashable. For GateInstruction, MeasurementInstruction this is what I have in mind

@dataclass(frozen=True)
class GateInstruction(CircuitInstruction):
    operation: Gate
    control_value: int | None = None

    def __post_init__(self):
        super().__post_init__()
        if len(self.qubits) != self.operation.num_qubits:
            raise ValueError(
                f"Gate '{self.operation.name}' requires {self.operation.num_qubits} qubits"
                f"But got {len(self.qubits)}."
            )

    def controls(self) -> Tuple[int]:
        return qubits[: self.operation.num_ctrl_qubits]

    def targets(self) -> Tuple[int]:
        return qubits[self.operation.num_ctrl_qubits: ]

@dataclass(frozen=True)
class MeasurementInstruction(CircuitInstruction):
    operation: Measurement

    def __post_init__(self):
        super().__post_init__()
        if len(self.qubits) != len(self.cbits):
            raise ValueError("Measurement requires equal number of qubits and cbits.")

    def __repr__(self):
        return f"Measure(q{self.qubits} -> c{self.cbits})"

[BoxiLi]

So in a sense this is equivalent to the old Gate class, which contains gate information plus information like qubits etc.

I like the use of frozen data classes. Under which scenario do we need to hash it?

[Mayank447]

Not equivalent, we can think of breaking old Gate class into two separate classes. Circuit instruction class for qubits, classical controls etc. and an abstract Gate class for qobj, gate properties like num_ctrl_qubits and gate decomposition functions.

[Mayank447]

Well hashing is mostly for future use but would be needed in case we want to add caching/memoization for transpiling or while compiling to pulses. Another benefit of hashing is to check circuit equivalence

[Mayank447]

@BoxiLi Not sure whether this is the right discussion tab as it is about gate refactoring. Is there a reason pulse-paper is a dir in doc/? (which is ideally meant for user guide and sphinx conf.py).

Could we may be shift it to its sole directory in root? This is the general practic for open source libraries e.g. quimb (https://github.com/jcmgray/quimb)

Also currently the paper examples are not being tested in GitHub workflows because of a bug in .yaml. I am mostly done with gate refactoring, some minor changes left, so moving towards fixing and updating the documentation.

[BoxiLi]

Yes, we can move it to a separate directory.

Also currently the paper examples are not being tested in GitHub workflows because of a bug in .yaml.
Oh no, that sounds bad. So it has been out of loop.

[BoxiLi]

Shall I merge #317 ？

[Mayank447]

Yes

[BoxiLi]

Originally, we put the paper code there just because we wanted it to be tested, together with the doc. You are right that doc is the place for the documentation. So indeed we should move it. Maybe it will also make sure that the paper code is properly tested and uptodate.

[Mayank447]

@BoxiLi Hi I have a different idea regarding how to manage ControlledGate, let me know your opinion on this.

Currently in #328 we are treating control_value like as an argument to the gate, so something like CZ(1). This causes several issue. First CZ.get_qobj() doesn't work because CZ needs to be initialised with the control_value. This is a little non-intuitive to the user. Second, in almost all the cases user expects the control_value to be 1 for almost all common gates and he doesn't really change the control_value of the gate in 99% of the cases. Third, for a gate the decomposition routine for Parametric Gates like RX($\theta$) works for all values of $\theta$, but the decomposition routine for say CCX (Toffoli) which works for the control_value 11 won't work for control_value 10. This is a problem as potentially we should move the decomposition routines to the gateclasses itself.

In order to simplify our class design and resolve the above, we could embed the control_value in the ControlledGate itself as a class attribute i.e. for all standard gates we assume the control_value is all 1s. A user can always define a given gate with his choice of controlled value in a single line actually using an existing function which I added.

gate = controlled(std.X, num_ctrl_qubits=1, ctrl_value=0)

This will also simplify the gateclass design for ControlledGate considerably and also make them much more general. So user can generated controlled version of his custom gates as well. gate = controlled(custom_gate, num_ctrl_qubits=1, ctrl_value=1). Currently certain nuances for ParametricGate like validate_params(), arguments for __init__ need to be handled in the ControlledGate, which is not a scalable design.

[BoxiLi]

Didn't see this message yesterday. They all sound good to me.