Fix #474: assertion fires in multicore mapper

[pablolh]

• Fix computation of is_comm_qubit in topology.cc

[pablolh]

Hi Roberto,

Originally I went for the least effort unit test that exposes the bug. In this new change it's better and I also integrated the precomputation of the number of qubits per core.

Pablo

[jvansomeren]

Hi Pablo,

Indeed the xy form just specifies that there is a coordinate system in which the qubits can be declared. The example is the 7 qubit starmon system:

0 1 / \ / \ 2 3 4 \ / \ / 5 6

that has x-coordinates 0 to 4 and y-coordinates 0 to 2; only every other grid point contains a qubit.

Similarly, edges are between specified pairs of qubits, not positions in the grid, and not defined by the underlying grid.

Originally there was a rectangular grid (with a qubit hat every grid point), and a diamond grid (like the starmon 7) which is like a rectangular grid but then 45 degrees rotated and then fit in an underlying grid; then there were odd rows (0 1) and (5 6) and even rows (2 3 4), odd columns (2), (3) and (4), and even columns (0 5) and 1 6) but that grew too complicated. Out of came the xy as it is now.

Best,

Hans

Op 2 mei 2023, om 10:33 heeft Pablo Le Hénaff @.***> het volgende geschreven:

@pablolh commented on this pull request.

---

In src/ql/com/tests/topology_test.cchttps://urldefense.com/v3/__https://github.com/QuTech-Delft/OpenQL/pull/475*discussion_r1182247138__;Iw!!PAKc-5URQlI!8ugPm_sq7Nj8cs48ODZXK4UICnsXe98N3icOEYMtc5WDN46u0bRrkkvoD77T9LGDj6ZdTckFNI0N8vxhkgnkMnHOl66sv2_z$:

+namespace com { + +class TopologyTest {}; + +TEST_CASE_FIXTURE(TopologyTest, "Single core grid 2x3") {

• std::uint64_t qubit_count = 6;

• +/*

• 0 --- 1 --- 2
• | | |
• 5 --- 4 --- 3

• +*/

• // This API is weird: I don't understand why "edges" need to be specified, when "form" is "xy".

@jvansomerenhttps://urldefense.com/v3/__https://github.com/jvansomeren__;!!PAKc-5URQlI!8ugPm_sq7Nj8cs48ODZXK4UICnsXe98N3icOEYMtc5WDN46u0bRrkkvoD77T9LGDj6ZdTckFNI0N8vxhkgnkMnHOlzcDpPVI$ do you reckon that specifying a grid structure should implicitely declare the grid qubit connectivity, not just coordinates?

— Reply to this email directly, view it on GitHubhttps://urldefense.com/v3/__https://github.com/QuTech-Delft/OpenQL/pull/475*pullrequestreview-1408639320__;Iw!!PAKc-5URQlI!8ugPm_sq7Nj8cs48ODZXK4UICnsXe98N3icOEYMtc5WDN46u0bRrkkvoD77T9LGDj6ZdTckFNI0N8vxhkgnkMnHOl2nb__vI$, or unsubscribehttps://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/AEDTBNTF3QXUJO4WR7RSTE3XEDBE3ANCNFSM6AAAAAAXRYWZFY__;!!PAKc-5URQlI!8ugPm_sq7Nj8cs48ODZXK4UICnsXe98N3icOEYMtc5WDN46u0bRrkkvoD77T9LGDj6ZdTckFNI0N8vxhkgnkMnHOl1mmhSeJ$. You are receiving this because you were mentioned.Message ID: @.***>

[pablolh]

Okay, then this was normal, I've removed the comment so this is ready for review

[rturrado]

Beautiful, Pablo, great work!

Thanks for adding the num_qubits_per_core stuff.

Single core tests I think will be helpful.

[rturrado]

Sorry, closed by mistake!