New transpiler techniques (gate optmization): U3GateFusion, RotationGateFusion and InverseCancellation#1606
New transpiler techniques (gate optmization): U3GateFusion, RotationGateFusion and InverseCancellation#1606carlos-luque wants to merge 11 commits intoqiboteam:masterfrom
Conversation
for more information, see https://pre-commit.ci
|
@alecandido is there a way for external PRs to trigger the CI and run the tests? |
alecandido
added
run-workflow
Yes, there is: it is similar to what you imagined. It is not the |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1606 +/- ##
==========================================
- Coverage 99.11% 99.05% -0.06%
==========================================
Files 77 77
Lines 11665 11802 +137
==========================================
+ Hits 11562 11691 +129
- Misses 103 111 +8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Thanks so much for your reviews, @renatomello. I am working on the code to add them |
- Implement unitary equivalence check between circuits - Renamed test functions to conform to PEP 8 snake_case style - Add tests for circuits with no gates and fused circuits
|
I included a usage example for new features in the documentation (/code-examples/advancedexamples.rst) |
renatomello
added
run-workflow
BrunoLiegiBastonLiegi
left a comment
BrunoLiegiBastonLiegi
left a comment
There was a problem hiding this comment.
Thanks for the very nice additions, I think there is the potential to do something quite nice, general and useful for future further developments. Namely, I think it may be worth implementing a general GateFuser class, as detailed in one of the comments below.
| return new | ||
|
|
||
|
|
||
| class InverseCancellation(Optimizer): |
There was a problem hiding this comment.
I was wondering if there was an easier (less convoluted and slightly more readable) way to implement this.
Namely, I would probably first identify which gates act on which qubits and build a mapping from the qubit to the index in the queue. Then for each qubit I would scan for gate pairs that cancel out and append them to the list of gates to remove (as you already do). Something like this:
@staticmethod
def _qubit_to_queue_index_map(circuit):
# build a mapping between qubit q --> [i1, i2, ..., in] indices of the
# gates in the queue that involve q
qmap = dict(range(circuit.nqubits), circuit.nquibts * [,])
for i, gate in enumerate(circuit.queue):
for q in gate.qubits:
qmap[q].append(i)
return qmap
def __call__(self, circuit):
qmap = self._qubit_to_queue_index_map(circuit)
gates = circuit.queue
# for each qubit look for cancelling pairs
for q in circuit.nqubits:
n = 0
# get the interested gates
indices = qmap[q]
# scan till the end
while n < len(indices):
# convert to queue indices
i, j = indices[n], indices[n+1]
# check if the gates cancel out
flag = self._same_gates(gates[i], gates[j])
if flag:
# if yes append to the gate to be removed
# and move two positions forward
self._gates_to_remove.update({i, j})
n += 2
else:
# if not just go ahead
n += 1
# build the new circuit
new_circuit = Circuit(circuit.nqubits, **circuit.init_kwargs)
for i, gate in enumerate(circuit.queue):
if not i in self._gates_to_remove:
new_circuit.add(gate)
return new_circuitThere was a problem hiding this comment.
This proposal is valid in general, but it fails in the presence of N-qubit gates.
For instance, consider a three-qubit circuit with the sequence: CNOT(0,1), X(1), Y(2), CNOT(0,1).
The transformation yields X(1) Y(2), which is incorrect.
| if isinstance(gate, self.rotated_gates): | ||
| prev_gate = previous_gates[primary_qubit] | ||
| if isinstance(prev_gate, gate.__class__): | ||
| tmp_gate = gate.__class__( | ||
| primary_qubit, prev_gate.parameters[0] + gate.parameters[0] | ||
| ) | ||
| previous_gates[primary_qubit] = tmp_gate | ||
| else: | ||
| if isinstance(prev_gate, self.rotated_gates): | ||
| self.gates.append(prev_gate) | ||
| previous_gates[primary_qubit] = gate | ||
| else: | ||
| # Flush stored rotations before adding new gate | ||
| for q in gate.init_args: | ||
| if isinstance(previous_gates[q], self.rotated_gates): | ||
| self.gates.append(previous_gates[q]) | ||
| previous_gates[q] = None | ||
|
|
||
| self.gates.append(gate) | ||
| for q in gate.qubits: | ||
| previous_gates[q] = gate |
There was a problem hiding this comment.
Can I ask you to add some comments here please? because I kind of get the idea but I am getting lost in all these nested checks. Even some more separation, in the sense of adding auxiliary functions with understandable names, would improve readability.
There was a problem hiding this comment.
The first and second conditions verify whether the gate pair consists of rotation operators of the same type.
If the second condition fails, the else branch checks whether the preceding gate (prev_gate) is a rotation operator of a different type from the current gate (gate).
In all other cases, the algorithm flushes the stored rotations and appends any non-rotation gates directly to the output sequence.
| def _merge_u3gates(self, nqubits: int, list_gates: list): | ||
| """Identify pairs of U3 gates that can be merged. | ||
|
|
||
| Args: | ||
| nqubits (int): number of qubits. | ||
| list_gates (list): a list of gates (:class:`qibo.gates.abstract.Gate`). | ||
| """ | ||
|
|
||
| previous_gates = [None] * nqubits | ||
|
|
||
| for gate in list_gates: | ||
|
|
||
| primary_qubit = gate.init_args[0] # Extract primary qubit | ||
|
|
||
| if isinstance(gate, gates.U3): | ||
| prev_gate = previous_gates[primary_qubit] | ||
| if isinstance(prev_gate, gates.U3): | ||
| tmp_gate = self._create_u3_fusion(primary_qubit, prev_gate, gate) | ||
| previous_gates[primary_qubit] = tmp_gate | ||
| else: | ||
| previous_gates[primary_qubit] = gate | ||
| else: | ||
| # Flush stored U3 before adding new gate | ||
| for q in gate.init_args: | ||
| if isinstance(previous_gates[q], gates.U3): | ||
| self.gates.append(previous_gates[q]) | ||
| previous_gates[q] = None | ||
|
|
||
| self.gates.append(gate) | ||
| for q in gate.qubits: | ||
| previous_gates[q] = gate # Track current gate | ||
|
|
||
| # Append any remaining U3 gates in one pass | ||
| self.gates.extend(g for g in previous_gates if isinstance(g, gates.U3)) |
There was a problem hiding this comment.
I think this is basically the same routine of the previous gate merger, I think you could avoid repeating this and write a generic class that searches and merges mergeable gates, naturally the rules of mergeability and search are going to be target specific. Now that I think about it, even the cancellation routine could be seen as a special case of this.
class GatesFuser(ABC, Optimizer):
# this will be totally generic
def __call__(self, circuit):
...
# these are object specific
@abstractmethod
def _are_fusable(g1, g2):
pass
@abstractmethod
def fuse(g1, g2):
pass
class InverseCancellation(GatesFuser)
...
class RotationFuser(GatesFuser)
...
class U3Fuser(GatesFuser):
...There was a problem hiding this comment.
The third nested condition verifies whether the preceding gate (prev_gate) is a rotation operator of a different type from the current gate (gate). This check is unnecessary in the case of a U3U3 gate merger.
I attempted to modify the routine following the approach proposed for inverse gate cancellation in order to simplify the implementation; however, this resulted in the loss of the original gate ordering within the circuit.
A generic class appears to be a promising design choice. However, in the current implementation, the only common generic method is call.
There was a problem hiding this comment.
A generic class appears to be a promising design choice. However, in the current implementation, the only common generic method is call.
Yeah and that is exactly what you want right? The __call__ method will be completely general and will loop over the circuit gates in search of fusable gates. For instance:
class GatesFuser(ABC, Optimizer):
def __init__(self):
self.gates = []
@property
@abstractmethod
def gate_type(self) -> Type:
pass
def __call__(self, circuit):
circuit = circuit.copy(True)
nqubits = circuit.nqubits
previous_gates = [None] * nqubits
for gate in circuit.queue:
primary_qubit = gate.init_args[0]
if isinstance(gate, self.gate_type):
prev_gate = previous_gates[primary_qubit]
if self.are_fusable(gate, prev_gate):
tmp_gate = self.fuse(gate, prev_gate)
previous_gates[primary_qubit] = tmp_gate
else:
self.not_fusable_post_processing(prev_gate)
previous_gates[primary_qubit] = gate
else:
# Flush stored gates before adding new gate
for q in gate.init_args:
if isinstance(previous_gates[q], self.gate_type):
self.gates.append(previous_gates[q])
previous_gates[q] = None
self.gates.append(gate)
for q in gate.qubits:
previous_gates[q] = gate
# Append any remaining gates in one pass
self.gates.extend(g for g in previous_gates if isinstance(g, self.gate_type))
new_circuit = circuit.__class__(**circuit.init_kwargs)
for gate in self.gates:
new_circuit.add(gate)
return new_circuit
@abstractmethod
def are_fusable(self, g1, g2):
pass
@abstractmethod
def fuse(self, g1, g2):
pass
def not_fusable_post_processing(self, previous_gate):
pass
class RotationFuser(GatesFuser):
def gate_type(self) -> Type:
return gates.RX | gates.RY | gates.RZ
def are_fusable(self, g1, g2):
return isinstance(g1, g2.__class__)
def fuse(self, g1, g2):
return g1.__class__(
g1.init_args[0], g1.parameters[0] + g2[0]
)
def not_fusable_post_processing(self, previous_gate):
if isinstance(previous_gate, self.gate_type):
self.gates.append(previous_gate)
class U3Fuser(GatesFuser):
def gate_type(self) -> Type:
return gates.U3
def are_fusable(self, g1, g2):
return isinstance(g2, gates.U3)
def fuse(self, g1, g2):
qubit = g1.init_args[0]
# your _create_u3_fusion
...
@staticmethod
def _extract_u3_params(unitary_matrix: np.ndarray):
...This is just a quick draft that I made, there surely are improvements to be made in several points, but roughly should give you the idea. The inverse cancellation can be similarly adapted to fit in this design with, in case, some further tuning of the abstract class I believe.
I would personally route for this solution (or something similar) due to it's way better maintainability, readability and flexibility.
|
@carlos-luque do you need a hand with this? |
|
Apologies for the delayed response. I will address it in the coming days. |
|
Hi @carlos-luque are you planning to complete this PR? |
6 participants
We have worked on issue #1588 and added new techniques in transpiler optimization:
Checklist: