Skip to content
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

Solovay Kitaev Decomposition doesn't correctly load saved approximations from file #12576

Closed
Ecpii opened this issue Jun 14, 2024 · 1 comment · Fixed by #12579
Closed

Solovay Kitaev Decomposition doesn't correctly load saved approximations from file #12576

Ecpii opened this issue Jun 14, 2024 · 1 comment · Fixed by #12579
Assignees
Labels
bug Something isn't working

Comments

@Ecpii
Copy link

Ecpii commented Jun 14, 2024

Environment

  • Qiskit version: 1.1.0
  • Python version: 3.10.11
  • Operating system: Windows 11 23H2
  • Numpy version: 1.26.4

What is happening?

After specifying a file to save the approximations of generate_basic_approximations into, attempting to load this into a new SolovayKitaevDecomposition gives a runtime error.

This is somewhat easily fixable by replacing this line with the following temporary fix:

        for gatestring, matrix in data.flat[0].items():

After this, the SolovayKitaevDecomposition object can be instantiated, but still contains a separate logic error. Some inputs can cause the circuits generated by this object to have different global phases than the ones generated by a SolovayKitaevDecomposition which did not load these approximations from a file (see example).

How can we reproduce the issue?

Minimal Example

from qiskit.synthesis.discrete_basis.solovay_kitaev import (
    SolovayKitaevDecomposition,
)
from qiskit.synthesis.discrete_basis import generate_basic_approximations

import numpy as np
from qiskit.synthesis.discrete_basis.gate_sequence import (
    _convert_so3_to_su2,
)

generate_basic_approximations(["t", "tdg", "h"], 10, "test.npy")
sk = SolovayKitaevDecomposition()
sk_file = SolovayKitaevDecomposition("test.npy")
# first issue occurs on line above

rotation = np.array([[0, 0, 1], [0, 1, 0], [-1, 0, 0]])

qc = sk.run(_convert_so3_to_su2(rotation), 2)
qc_file = sk_file.run(_convert_so3_to_su2(rotation), 2)

print(f"{qc.global_phase = }")
print(f"{qc_file.global_phase = }")

print(f"qc =\n{qc.draw()}")
print(f"qc_file =\n{qc_file.draw()}")

Output

Runtime Error on loading file

Traceback (most recent call last):
  File "[omitted]\saved_approximations.py", line 13, in <module>
    sk_file = SolovayKitaevDecomposition("test.npy")    
  File "[omitted]\env\lib\site-packages\qiskit\synthesis\discrete_basis\solovay_kitaev.py", line 54, in __init__
    self.basic_approximations = self.load_basic_approximations(basic_approximations)
  File "[omitted]\env\lib\site-packages\qiskit\synthesis\discrete_basis\solovay_kitaev.py", line 80, in load_basic_approximations
    for gatestring, matrix in data.items():
AttributeError: 'numpy.ndarray' object has no attribute 'items'. Did you mean: 'item'?

After Runtime Error Fix

qc.global_phase = 3.141592653589793
qc_file.global_phase = 0.0
qc =
global phase: π
   ┌───┐┌───┐┌───┐┌───┐┌───┐
q: ┤ H ├┤ T ├┤ T ├┤ T ├┤ T ├
   └───┘└───┘└───┘└───┘└───┘
qc_file =
   ┌───┐┌───┐┌───┐┌───┐┌───┐
q: ┤ H ├┤ T ├┤ T ├┤ T ├┤ T ├
   └───┘└───┘└───┘└───┘└───┘ 

What should happen?

The circuit created using the SolovayKitaevDecomposition created with saved approximations (qc_file) should behave the same as the one without (qc) - they should both give the same result gates and global phase, as well as run without runtime errors.

Any suggestions?

  • The temporary fix for the AttributeError issue should probably not be used (or at least checked before used). On my system, at least, the value of data at line 79 in solovay_kitaev.py is a 0 dimensional np.ndarray containing the object loaded from the file. I am not sure whether this is consistent on other systems or numpy versions.
    • Also, data.size is not checked in this one-line solution, so there could be some sort of security vulnerability if this is used.
  • It seems that only the gates and product are saved into the file (source code here). It is also mentioned explicitly in the constructor docstring of SolovayKitaevDecomposition, so I'm unsure if this is intentional. If not and this needs to be changed, this documentation should also be updated.
@Cryoris
Copy link
Contributor

Cryoris commented Jun 14, 2024

Thanks for reporting this! Your fix was almost there; by unwrapping the numpy array upon loading the npy file we can fix the behavior without changing the normal loading path. #12579 should fix this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants