[CQT-243] Implementation of global phase #563

juanboschero · 2025-07-09T08:14:20Z

Changed circuit class
Added PhaseMap class
Changed internal decomposer logic
Added helper functions
Add tests

elenbaasc

Thanks for the effort!

I think that the PhaseMap could simply track the phase angle (so no need for an array of complex values).

Also, the phase map still needs a separate test module.

And don't forget to add an entry to the Changelog.

opensquirrel/phasemap.py

opensquirrel/circuit.py

elenbaasc · 2025-07-17T08:16:43Z

opensquirrel/common.py

    return are_matrices_equivalent_up_to_global_phase(matrix, np.eye(matrix.shape[0], dtype=np.complex128))
+
+
+def get_phase_angle(scalar: np.complex128) -> np.complex128:


The phase angle will never be complex, right? It's just a float value.

I guess it could also be a simply built-in float, instead of a numpy.float ... right?

opensquirrel/ir.py

opensquirrel/passes/decomposer/general_decomposer.py

elenbaasc · 2025-07-17T10:34:08Z

opensquirrel/phasemap.py

+        """Checks if qubit is in the phase map."""
+        return qubit in self.qubit_phase_map
+
+    def add_qubit_phase(self, qubit: QubitLike, phase: np.complex128) -> None:


Suggested change

def add_qubit_phase(self, qubit: QubitLike, phase: np.complex128) -> None:

def set_qubit_phase(self, qubit: QubitLike, phase: np.complex128) -> None:

opensquirrel/phasemap.py

tests/integration/test_integration.py

Co-authored-by: Chris Elenbaas <[email protected]>

elenbaasc

Thanks for making the adjustments!

Mainly the tests need to be updated. It is not clear what the function is of the phase map (at least not from the tests). There needs to be a test that explicitly shows that the compiled circuit has been adjusted/corrected for phase differences between interacting qubits. Current tests don't show/test this.

Other than that, there are still some minor changes and some comments still left from the previous review round.

elenbaasc · 2025-07-25T12:26:23Z

opensquirrel/phase_map.py

+    def __init__(self, qubit_register_size: int) -> None:
+        """Initialize a PhaseMap object."""
+
+        self.qubit_phase_map = np.zeros(qubit_register_size, dtype=np.float64)


Any reason why this needs to be a numpy array?

Suggested change

self.qubit_phase_map = np.zeros(qubit_register_size, dtype=np.float64)

self.qubit_phase_map = [0] * qubit_register_size

elenbaasc · 2025-07-25T12:29:28Z

opensquirrel/phase_map.py

+from opensquirrel.ir import Qubit, QubitLike
+
+
+class PhaseMap:


It is more a PhaseRecord instead. It does not 'map' from a domain to a range, but it records the value of the phase as you pass through the circuit. Replace 'map' -> 'record' in the rest of the code.

elenbaasc · 2025-07-25T12:43:58Z

tests/integration/test_integration.py

+Rz(0.78539824) q[1]
 X90 q[1]
-Rz(-3.1415927) q[1]
+Rz(1.5707963) q[1]
+X90 q[1]
+Rz(1.5707963) q[1]


These changes are due to the change made in the in check_gate_replacement where in the conditional statement if is_identity_matrix_up_to_a_global_phase(replaced_matrix): now an empty list is returned, whereas before nothing was returned. So, after the first decomposition pass (SWAP2CNOTDecomposer) the Rz(tau) q[1] is removed from the circuit (because it is identity). This is unwanted behaviour, we should not want a 2-qubit decomposition pass to process single-qubit gates, even if they are identity.

Moreover, these differences are not due to the correction of a phase difference. If you remove the Rz(tau) q[1], the compiled circuit you end up with is the same with the 'phase map' functionality, as it is without.

elenbaasc · 2025-07-25T12:48:32Z

tests/test_phase_map.py

+from opensquirrel.passes.merger import SingleQubitGatesMerger
+
+
+def test_integration_global_phase() -> None:


This test is too high level to test the integration of the global phase and also the resulting (compiled) circuit is the same without the phase map functionality (same output, if this test is run on develop).

A test that specifically tests the functionality of the phase map is needed, whereby the resulting circuit has been corrected for the global phase difference (i.e., the compiled circuit should look different if run on current develop branch and be incorrect (semantically))

I made it a bit simpler and explicitly left it as a high level test

elenbaasc · 2025-07-25T12:53:56Z

tests/test_phase_map.py

+    )
+
+
+def test_phase_map_circuit_builder() -> None:


The creation and evolution of the phase map does not depend on whether the parser or circuit builder is used, so it is no need to test it explicitly.

This test does show that the phase map is updated (although not in a very predictable way). Is they expected result obvious, given the circuit and compile passes used? Would be better to make this more simply and have a couple of different circuits, resulting in different values for the recorded phases.

I have made it smaller, I realized now that there was a blatant error in the decomposer. I was modifying the list without storing it (I forgot that Python does not implicitly do deep copies). You will now see how this impacts the rest of our code.

elenbaasc · 2025-07-25T12:55:07Z

opensquirrel/common.py

    return are_matrices_equivalent_up_to_global_phase(matrix, np.eye(matrix.shape[0], dtype=np.complex128))
+
+
+def get_phase_angle(scalar: np.complex128) -> np.complex128:


I guess it could also be a simply built-in float, instead of a numpy.float ... right?

elenbaasc · 2025-07-25T13:00:02Z

opensquirrel/phase_map.py

+    def add_qubit_phase(self, qubit: QubitLike, phase: np.float64) -> None:
+        self.qubit_phase_map[Qubit(qubit).index] += phase
+
+    def get_qubit_phase(self, qubit: QubitLike) -> np.float64:
+        return np.float64(self.qubit_phase_map[Qubit(qubit).index])


Suggested change

def add_qubit_phase(self, qubit: QubitLike, phase: np.float64) -> None:

self.qubit_phase_map[Qubit(qubit).index] += phase

def get_qubit_phase(self, qubit: QubitLike) -> np.float64:

return np.float64(self.qubit_phase_map[Qubit(qubit).index])

def add_qubit_phase(self, qubit: QubitLike, phase: float) -> None:

self.qubit_phase_map[Qubit(qubit).index] += phase

def get_qubit_phase(self, qubit: QubitLike) -> float:

return self.qubit_phase_map[Qubit(qubit).index]

Fix tests and update docs.

init

d0b0d2e

juanboschero requested a review from elenbaasc July 9, 2025 08:14

Reinstate type ignore.

c0d4f62

elenbaasc reviewed Jul 17, 2025

View reviewed changes

juanboschero and others added 3 commits July 18, 2025 08:06

Update on review and add more tests

92bf58c

Apply suggestions from code review

c91d0eb

Co-authored-by: Chris Elenbaas <[email protected]>

happify ruff

4c0a429

elenbaasc reviewed Jul 25, 2025

View reviewed changes

juanboschero and others added 6 commits July 28, 2025 12:01

fix errors

04208c6

happify poetry

25c94f3

Fix hectoqubit test.

25684a3

Merge develop in feature branch.

e58095b

Merge develop into feature branch.

14499b8

Fix tests and update docs.

fix conflicts

b93d584

		return are_matrices_equivalent_up_to_global_phase(matrix, np.eye(matrix.shape[0], dtype=np.complex128))


		def get_phase_angle(scalar: np.complex128) -> np.complex128:

	def add_qubit_phase(self, qubit: QubitLike, phase: np.complex128) -> None:
	def set_qubit_phase(self, qubit: QubitLike, phase: np.complex128) -> None:

	self.qubit_phase_map = np.zeros(qubit_register_size, dtype=np.float64)
	self.qubit_phase_map = [0] * qubit_register_size

		from opensquirrel.passes.merger import SingleQubitGatesMerger


		def test_integration_global_phase() -> None:

[CQT-243] Implementation of global phase #563

Are you sure you want to change the base?

[CQT-243] Implementation of global phase #563

Uh oh!

Conversation

juanboschero commented Jul 9, 2025

Uh oh!

elenbaasc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

elenbaasc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants