-
Notifications
You must be signed in to change notification settings - Fork 4
[CQT-43] Take global phases into account when verifying decompositions #368
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -2,11 +2,15 @@ | |||||||||||
|
||||||||||||
from collections.abc import Callable | ||||||||||||
from typing import TYPE_CHECKING, Any | ||||||||||||
from dataclasses import dataclass | ||||||||||||
|
||||||||||||
import numpy as np | ||||||||||||
|
||||||||||||
from opensquirrel.passes.exporter.export_format import ExportFormat | ||||||||||||
|
||||||||||||
if TYPE_CHECKING: | ||||||||||||
from opensquirrel.ir import IR, Gate | ||||||||||||
from numpy.typing import ArrayLike, NDArray | ||||||||||||
from opensquirrel.ir import IR, Gate, QubitLike | ||||||||||||
from opensquirrel.passes.decomposer import Decomposer | ||||||||||||
from opensquirrel.passes.mapper import Mapper | ||||||||||||
from opensquirrel.register_manager import RegisterManager | ||||||||||||
|
@@ -99,7 +103,7 @@ def decompose(self, decomposer: Decomposer) -> None: | |||||||||||
""" | ||||||||||||
from opensquirrel.passes.decomposer import general_decomposer | ||||||||||||
|
||||||||||||
general_decomposer.decompose(self.ir, decomposer) | ||||||||||||
general_decomposer.decompose(self, decomposer) | ||||||||||||
|
||||||||||||
def map(self, mapper: Mapper) -> None: | ||||||||||||
"""Generic qubit mapper pass. | ||||||||||||
|
@@ -116,7 +120,7 @@ def replace(self, gate_generator: Callable[..., Gate], f: Callable[..., list[Gat | |||||||||||
""" | ||||||||||||
from opensquirrel.passes.decomposer import general_decomposer | ||||||||||||
|
||||||||||||
general_decomposer.replace(self.ir, gate_generator, f) | ||||||||||||
general_decomposer.replace(self, gate_generator, f) | ||||||||||||
|
||||||||||||
def export(self, fmt: ExportFormat | None = None) -> Any: | ||||||||||||
if fmt == ExportFormat.QUANTIFY_SCHEDULER: | ||||||||||||
|
@@ -129,3 +133,31 @@ def export(self, fmt: ExportFormat | None = None) -> Any: | |||||||||||
return cqasmv1_exporter.export(self) | ||||||||||||
msg = "unknown exporter format" | ||||||||||||
raise ValueError(msg) | ||||||||||||
|
||||||||||||
|
||||||||||||
@dataclass(init=False) | ||||||||||||
class PhaseMap(Circuit): | ||||||||||||
|
||||||||||||
def __init__(self, phase_map: ArrayLike[np.complex128]): | ||||||||||||
"""Initialize a PhaseMap object.""" | ||||||||||||
self.phase_map = phase_map | ||||||||||||
Comment on lines
+141
to
+143
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems unnecessary for a 'dataclass', normally if the properties of the dataclass need to be preprocessed, one would use the |
||||||||||||
|
||||||||||||
def __contains__(self, qubit: QubitLike) -> bool: | ||||||||||||
"""Checks if qubit is in the phase map.""" | ||||||||||||
if qubit in self.phase_map: | ||||||||||||
return True | ||||||||||||
|
||||||||||||
return False | ||||||||||||
Comment on lines
+147
to
+150
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||
|
||||||||||||
def set_phase_map(self, phase_map: NDArray[np.complex128]) -> None: | ||||||||||||
self.qubit_phase_map = phase_map | ||||||||||||
Comment on lines
+152
to
+153
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Setter and getter functionality is sort of built-in in dataclasses, that's there purpose. So this is either not needed, or this should be a normal class, where the |
||||||||||||
|
||||||||||||
Comment on lines
+152
to
+154
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
I think this is a copy construction, and should be taken care of in the/a constructor. |
||||||||||||
def add_qubit_phase(self, qubit: QubitLike, phase: np.complex128) -> None: | ||||||||||||
from opensquirrel.ir import Qubit | ||||||||||||
|
||||||||||||
self.qubit_phase_map[Qubit(qubit).index] += phase | ||||||||||||
|
||||||||||||
def get_qubit_phase(self, qubit: QubitLike) -> np.complex128: | ||||||||||||
from opensquirrel.ir import Qubit | ||||||||||||
|
||||||||||||
return np.complex128(self.qubit_phase_map[Qubit(qubit).index]) |
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -39,13 +39,39 @@ def are_matrices_equivalent_up_to_global_phase( | |||||||||
Returns: | ||||||||||
Whether two matrices are equivalent up to a global phase. | ||||||||||
""" | ||||||||||
phase_difference = calculate_phase_difference(matrix_a, matrix_b) | ||||||||||
|
||||||||||
if not phase_difference: | ||||||||||
return False | ||||||||||
|
||||||||||
return np.allclose(matrix_a, phase_difference * matrix_b) | ||||||||||
|
||||||||||
|
||||||||||
def calculate_phase_difference(matrix_a: NDArray[np.complex128], matrix_b: NDArray[np.complex128]) -> np.complex128: | ||||||||||
"""Calculates the phase difference between two matrices. | ||||||||||
|
||||||||||
Args: | ||||||||||
matrix_a: first matrix. | ||||||||||
matrix_b: second matrix. | ||||||||||
|
||||||||||
Returns: | ||||||||||
The phase difference between the two matrices. | ||||||||||
""" | ||||||||||
first_non_zero = next( | ||||||||||
(i, j) for i in range(matrix_a.shape[0]) for j in range(matrix_a.shape[1]) if abs(matrix_a[i, j]) > ATOL | ||||||||||
) | ||||||||||
|
||||||||||
if abs(matrix_b[first_non_zero]) < ATOL: | ||||||||||
return False | ||||||||||
return np.complex128(1) | ||||||||||
|
||||||||||
phase_difference = matrix_a[first_non_zero] / matrix_b[first_non_zero] | ||||||||||
return np.complex128(matrix_a[first_non_zero] / matrix_b[first_non_zero]) | ||||||||||
|
||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice! |
||||||||||
return np.allclose(matrix_a, phase_difference * matrix_b) | ||||||||||
|
||||||||||
def to_euler_form(scalar: np.complex128) -> np.complex128: | ||||||||||
""" " Derives the Euler rotation angle from a scalar. | ||||||||||
Comment on lines
+70
to
+71
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
Args: | ||||||||||
scalar: scalar to convert. | ||||||||||
Returns: | ||||||||||
Euler phase angle of scalar. | ||||||||||
""" | ||||||||||
return np.complex128(-1j * np.log(scalar)) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -113,6 +113,75 @@ def test_Spin2_backend() -> None: # noqa: N802 | |
) | ||
|
||
|
||
def test_integration_global_phase() -> None: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we add a comment somewhere in the code saying where a global phase is expected to be introduced? |
||
qc = Circuit.from_string( | ||
""" | ||
version 3.0 | ||
|
||
qubit[3] q | ||
|
||
H q[0:2] | ||
Ry(1.5789) q[0] | ||
H q[0] | ||
CNOT q[1], q[0] | ||
Ry(3.09) q[0] | ||
Ry(0.318) q[1] | ||
Ry(0.18) q[2] | ||
CNOT q[2], q[0] | ||
""", | ||
) | ||
|
||
# Decompose 2-qubit gates to a decomposition where the 2-qubit interactions are captured by CNOT gates | ||
qc.decompose(decomposer=CNOTDecomposer()) | ||
|
||
# Replace CNOT gates with CZ gates | ||
qc.replace( | ||
CNOT, | ||
lambda control, target: [ | ||
H(target), | ||
CZ(control, target), | ||
H(target), | ||
], | ||
) | ||
|
||
# Merge single-qubit gates and decompose with McKay decomposition. | ||
qc.merge_single_qubit_gates() | ||
qc.decompose(decomposer=McKayDecomposer()) | ||
|
||
assert ( | ||
str(qc) | ||
== """version 3.0 | ||
|
||
qubit[3] q | ||
|
||
Rz(1.5707963) q[1] | ||
X90 q[1] | ||
Rz(1.5707963) q[1] | ||
Rz(3.1415927) q[0] | ||
X90 q[0] | ||
Rz(0.0081036221) q[0] | ||
X90 q[0] | ||
Rz(3.1415927) q[0] | ||
CZ q[1], q[0] | ||
X90 q[2] | ||
Rz(1.3907963) q[2] | ||
X90 q[2] | ||
X90 q[0] | ||
Rz(0.051592695) q[0] | ||
X90 q[0] | ||
Rz(3.1415927) q[0] | ||
CZ q[2], q[0] | ||
Rz(1.5707963) q[0] | ||
X90 q[0] | ||
Rz(1.5707963) q[0] | ||
Rz(3.1415927) q[1] | ||
X90 q[1] | ||
Rz(2.8235927) q[1] | ||
X90 q[1] | ||
Comment on lines
+155
to
+180
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just out of curiosity, what instructions, if any, do the global phase correction here? |
||
""" | ||
) | ||
|
||
|
||
def test_hectoqubit_backend() -> None: | ||
qc = Circuit.from_string( | ||
""" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is quite an odd construction: a dataclass that subclasses from a normal class. Also, why would a PhaseMap have a 'is a' relationship with a Circuit, i.e., it is not clear why a PhaseMap would subclass from a Circuit (just like it is not clear why a Parser 'is a' InstructionLibrary)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not that it's not clear. It's wrong. It's a wrong way to get access to information from the base class. But inheritance should only be used when there is an is a relationship with the base class.
A PhaseMap should be an independent data structure, managing some data, e.g., a map of phases, encapsulating the definition of that map of phases (today a dictionary, tomorrow whatever), and providing a given API to access, query, etc. that data.
If PhaseMap turns out to be very simple, e.g., just a map over which we only do set and get, we can decide not to use it. But at the very first moment that it is more complex than that, you won't want to be writing complicating code all around the code base to access a map of phases. In that case, we'd better have a PhaseMap.