SABRE + CircuitMap Optimization

[csookim]

To improve Sabre's performance, several implementations have been updated. I’ve added comments to the code and each comment number corresponds to the explanation numbers provided below. Please let me know if any clarifications are needed. Below is the code explanation.

Checklist:

• Reviewers confirm new code works as expected.
• Tests are passing.
• Coverage does not decrease.
• Documentation is updated.

After the code review:

• Delete comments.
• Update test functions.
• Optimize _swap_candidates(), _check_execution(), _shortest_path_routing() (remove get_physical_qubits(block_num)).
• Optimize ShortestPaths.

[csookim]

SABRE + CircuitMap Optimization

Previous Implementation

• The mapping scheme could be clearer.
• Using list.index() in logical-to-physical and circuit-to-physical mapping slowed down the routing.
• Frequently using the function that finds a gate's target qubits by block number was inefficient.

New Implementation

The code has been updated in three key areas:

1. Qubit Mapping

• SABRE operates using connectivity, initial_layout, and circuit.
• initial_layout is a dictionary where "q{physical_qubit_number}": {logical_qubit_number} pairs define the initial mapping.
• physical_qubit_number corresponds to the node number in connectivity.
• The initial_layout and connectivity can have arbitrary physical_qubit_numbers, when using restricted connectivity.

1-1. Layout Relabel

• To simplify routing, initial_layout and connectivity are relabeled before routing begins.
• physical_qubit_number is reassigned to a range from 0 to len(connectivity.nodes) - 1.
• For example, the layout {"q4":1, "q6":2, "q3":0} might be relabeled to {"q0":1, "q1":2, "q2":0}.
• This relabeling map is saved, and the final layout is reverted to the original labeling when routing is complete.
• All operations in Sabre are performed using the updated physical_qubit_number.

qibo/src/qibo/transpiler/router.py

Lines 731 to 739 in 0111a6e

	# 1# To simplify routing, some data is relabeled before routing begins.
	# 1# physical qubit is reassigned to a range from 0 to len(self.connectivity.nodes) - 1.
	node_mapping, new_initial_layout = {}, {}
	for i, node in enumerate(self.connectivity.nodes):
	node_mapping[node] = i
	new_initial_layout["q" + str(i)] = initial_layout["q" + str(node)]
	self.connectivity = nx.relabel_nodes(self.connectivity, node_mapping)
	self.node_mapping_inv = {v: k for k, v in node_mapping.items()}

qibo/src/qibo/transpiler/router.py

Lines 702 to 707 in 0111a6e

	# 1# final layout is reverted to the original labeling
	final_layout = self.circuit.final_layout()
	final_layout_restored = {
	"q" + str(self.node_mapping_inv[int(k[1:])]): v
	for k, v in final_layout.items()
	}

1-2 Physical-Logical Mapping

• Due to the layout labeling, there are two sets of numbers ranging from 0 to len(connectivity.nodes()) - 1 in initial_layout.
• Physical to logical mapping (p2l) and logical to physical mapping (l2p) are implemented using two arrays: self._p2l and self._l2p.
• For example, with a mapping of {"q0":1, "q1":2, "q2":0}, self._p2l would be [1, 2, 0] and self._l2p would be [2, 0, 1].
• Routing is performed using these p2l and l2p conversions.

qibo/src/qibo/transpiler/router.py

Lines 183 to 192 in 0111a6e

	# 1# bidirectional mapping
	# 1# self._p2l: physical qubit number i -> logical qubit number _p2l[i]
	# 1# self._l2p: logical qubit number i -> physical qubit number _l2p[i]
	self._l2p, self._p2l = [0] * len(self.initial_layout), [0] * len(
	self.initial_layout
	)
	for mapping in self.initial_layout.items():
	physical_qubit, logical_qubit = int(mapping[0][1:]), mapping[1]
	self._l2p[logical_qubit] = physical_qubit
	self._p2l[physical_qubit] = logical_qubit

qibo/src/qibo/transpiler/router.py

Lines 280 to 284 in 0111a6e

	# 1# update the bidirectional mapping
	p1, p2 = swap_p
	l1, l2 = swap_l
	self._p2l[p1], self._p2l[p2] = l2, l1
	self._l2p[l1], self._l2p[l2] = p2, p1

2. Temporary CircuitMap

• Creating a CircuitMap includes unnecessary operations such as block decomposition, circuit object creation, and gate copying, which are not needed for calculating the cost of a candidate swap.
• Enable temporary CircuitMap using a self._temporary flag.
• For temporary CircuitMap instances used only to calculate swap costs, return immediately from init after initializing self._l2p and self._p2l.
• In these temporary CircuitMap instances, skip gate additions or removals in the update() method.

qibo/src/qibo/transpiler/router.py

Lines 194 to 196 in 0111a6e

	self._temporary = temp
	if self._temporary: # 2# if temporary circuit, no need to store the blocks
	return

qibo/src/qibo/transpiler/router.py

Lines 843 to 849 in 0111a6e

	# 2# use CircuitMap for temporary circuit to save time
	# 2# no gates, no block decomposition, no Circuit object
	# 2# just logical-physical mapping
	temporary_circuit = CircuitMap(
	initial_layout=self.circuit.initial_layout,
	temp=True,
	)

3. Blocks in the front_layer

• SABRE calculates the cost of a swap by summing the distances of successive gates.
• To calculate the distance of a successive gate, the target qubit numbers of the gate need to be given.
• Previously, only gate (block) numbers were stored in the front_layer.
• To find the target qubits, the code had to iterate through the list of gates using search_by_index(block), which increased runtime.
• The new approach stores both the gate number and the target qubits when creating self._dag.

qibo/src/qibo/transpiler/router.py

Lines 990 to 995 in 0111a6e

	dag = nx.DiGraph()
	dag.add_nodes_from(range(len(gates_qubits_pairs)))
	# 3# additionally store target qubits of the gates
	for i in range(len(gates_qubits_pairs)):
	dag.nodes[i]["qubits"] = gates_qubits_pairs[i]

• For example,

self._dag = [(0, {'qubits': (0, 9), 'layer': 0}), (1, {'qubits': (5, 9), 'layer': 1}), (2, {'qubits': (2, 8), 'layer': 0})]

• The function _get_dag_layer(n, qubits=True) returns a list of target qubits for the gates in layer n.
• This allows retrieval of target qubits in $O(1)$ time.

qibo/src/qibo/transpiler/router.py

Lines 807 to 815 in 0111a6e

	# 3# depend on the 'qubits' flag, return the block number or target qubits
	# 3# return target qubits -> to avoid using get_physical_qubits(block_num)
	if qubits:
	layer_qubits = []
	nodes = self._dag.nodes(data=True)
	for node in nodes:
	if node[1]["layer"] == n_layer:
	# return target qubits
	layer_qubits.append(node[1]["qubits"])

4. Additional Improvements

• Use shallow copy when copying integer lists.

Evaluation

• Test: 100 random 50-CZ gates on 20 qubits.
• Line connectivity
• Qibo SABRE is 6x faster than before.
• The reason for the reduction in the number of SWAP gates is unknown.

                     Mean   Std
Qiskit Sabre Time    0.0439 0.0075
Qibo Sabre Time      0.0625 0.0179
Qibo Sabre Old Time  0.4252 0.106

Qiskit Sabre Swaps   189.97 21.7295
Qibo Sabre Swaps     196.54 21.8712
Qibo Sabre Old Swaps 279.96 35.739

[BrunoLiegiBastonLiegi]

Thanks @csookim, I looked into the new CircuitMap and it seems better, I just have a handful of comments.

[Simone-Bordoni]

Thank you for this improvement, I had a first look at the changes and I agree on the corrections proposed by @BrunoLiegiBastonLiegi
Regarding the performance improvement (number of swaps) I beliave that refactoring the circuit_map probaly solved some bug that we were not able to identify.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.11%. Comparing base (434989e) to head (24ecc7a).
Report is 26 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1419   +/-   ##
=======================================
  Coverage   97.10%   97.11%           
=======================================
  Files          81       81           
  Lines       11653    11679   +26     
=======================================
+ Hits        11316    11342   +26     
  Misses        337      337           

Flag	Coverage Δ
unittests	97.11% <100.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[BrunoLiegiBastonLiegi]

Thanks @csookim, this looks almost perfect to me now, I just made some further minor comments.
Regarding the coverage, there are 4 lines missing, some are related to the error of the circuit not being passed to the CircuitMap, which as I pointed out previously it would better to drop by making the argument mandatory. Some others to one of the two new properties setter that it's probably never used.

[csookim]

Custom qubit names can be used in Sabre. But the node names in the connectivity graph and the physical qubit names in the layout must be identical. Layout relabeling and recovery are done within CircuitMap.

CircuitMap

• Receive the layout {'A': 2, 'B': 0, 'C': 1}.
• Assign physical qubit numbers to the keys of the layout ('A' -> 0, 'B' -> 1, 'C' -> 2).
• Relabel the connectivity graph based on this mapping.
• Save the qubit names (keys) in wire_names.
• Logical qubit numbers are the values of the layout.
• Route using physical and logical qubit numbers.
• Restore the original connectivity graph using the inverse of the mapping (0 -> 'A', 1 -> 'B', 2 -> 'C').

Regarding the test files, some placers currently use node names that do not match the physical qubit names.

qibo/src/qibo/transpiler/placer.py

Lines 203 to 205 in 21732ad

	:math:`{\\textup{"q0"}: 1, \\textup{"q1"}: 2, \\textup{"q2"}:0}`
	to assign the physical qubits :math:`\\{0, 1, 2\\}`
	to the logical qubits :math:`[1, 2, 0]`.

I believe some parts of the code in Placer need to be updated, and the test files should be updated after resolving issue #1429.

[BrunoLiegiBastonLiegi]

Thanks @csookim for the insight about the wire_names, indeed as this PR is already adding various optimizations to the CircuitMap object, I am ok with merging this with the support for the default naming scheme only (q0, q1, ...), as long as we open an issue to remind us about it. So, in the end, up to you, if you feel this is something straightforward we could address it here directly, fine, otherwise we leave it for a future PR. In any case the support to custom qubit names is probably not a widely used feature anyway...

[Simone-Bordoni]

Thanks @csookim for the insight about the wire_names, indeed as this PR is already adding various optimizations to the CircuitMap object, I am ok with merging this with the support for the default naming scheme only (q0, q1, ...), as long as we open an issue to remind us about it. So, in the end, up to you, if you feel this is something straightforward we could address it here directly, fine, otherwise we leave it for a future PR. In any case the support to custom qubit names is probably not a widely used feature anyway...

At the moment it is not an important requirement but it may become fundamental for qibolab in the future when it will support hardware qubit names different from integers.

[Simone-Bordoni]

@csookim there are a few tests failing in the pipeline and router. If you can fix them then i will proceed with the review

[BrunoLiegiBastonLiegi]

I just did, there are some open comments that were not addressed and did not receive any answer

[BrunoLiegiBastonLiegi]

@csookim I don't see any new updates or answers to the points that are still open, could you please double check those and the tests that are failing?
Regarding the custom wires_names, as agreed, we could disentagle that from this PR and address it in a separate one, just please open an issue about that as a reminder.

[BrunoLiegiBastonLiegi]

@BrunoLiegiBastonLiegi Could you close these reviews if resolved? If you need further clarification, please let me know.

I cannot see any answer to those comments, did you answer them? Please double check, because if you click the button "start a review" instead of "Add a single comment", your answers are going to be pending until you finish the review, and they are going to be visible for you only.

[csookim]

@BrunoLiegiBastonLiegi I've updated the code handling these issues and modified the test functions.

Ah I see, then I believe it would be better to move the circuit check right after the temporary one:

        if self._temporary:  # if temporary circuit, no need to store the blocks
            return
        elif circuit is None:
            raise_error(ValueError, "Circuit must be provided for a non temporary `CircuitMap`.")
        self._nqubits = circuit.nqubits  # number of qubits

Except for the wire_names, I believe the case of the helper function is still open

Currently, the Router operates on physical qubits qi in the layout, which correspond to i in the connectivity nodes. Once all of your reviews are resolved, I will remove the comments.

[BrunoLiegiBastonLiegi]

Thanks @csookim this looks ready to me know, could you just add a reminder to #1377 about the wire_names?

[Simone-Bordoni]

I agree with the PR, However after the latest merge with master there is an error occurring. Can you fix it before I approve?

[csookim]

I agree with the PR, However after the latest merge with master there is an error occurring. Can you fix it before I approve?

There is a bug in drawer #1438. I will update it as soon as the issue is resolved. Please approve it after the update.

[csookim]

@Simone-Bordoni Please review this PR. Thanks.