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.

Sign up for GitHub

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

Add more classes for 2q synthesis to qiskit synthesis docs #11687

Merged
merged 5 commits into from
Feb 1, 2024
Merged

Add more classes for 2q synthesis to qiskit synthesis docs #11687

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e694913
add more clases to qiskit/synthesis init file
ShellyGarion Jan 31, 2024
5776562
improve TwoQubitWeilDecomposition docs
ShellyGarion Jan 31, 2024
7eb0bbe
Apply suggestions from code review
mtreinish Feb 1, 2024
b5e296e
Only make two_qubit_cnot_decompose and TwoQubitWeylDecomposition public
mtreinish Feb 1, 2024
613c5c3
Merge branch 'main' into docs_2q_synth
mtreinish Feb 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion qiskit/synthesis/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@

TwoQubitBasisDecomposer
XXDecomposer
TwoQubitWeylDecomposition

"""

Expand Down Expand Up @@ -162,4 +163,8 @@
from .unitary import aqc
from .one_qubit import OneQubitEulerDecomposer
from .two_qubit.xx_decompose import XXDecomposer
from .two_qubit.two_qubit_decompose import TwoQubitBasisDecomposer, two_qubit_cnot_decompose
from .two_qubit.two_qubit_decompose import (
TwoQubitBasisDecomposer,
two_qubit_cnot_decompose,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this two_qubit_cnot_decompose is missing an .. autofunction:: in the docstring? If it's meant to be there, let's take a note to add it after rc1 - no need to hold up the PR over a documentation thing.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, yeah good catch I missed that in the original review. I'll open an issue to track this. It actually will likely require a full docstring addition as it's doesn't have anything there now: https://github.com/Qiskit/qiskit/blob/main/qiskit/synthesis/two_qubit/two_qubit_decompose.py#L1553

it's a lazy loaded instance of TwoQubitBasisDecomposer using cx.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually already opened #11709 for it - forgot to comment here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also wasn't sure what to do with this function, since it has no docstring, and also due to the comment here:

qiskit/qiskit/synthesis/two_qubit/two_qubit_decompose.py

Line 1531 in a234ff2

# This weird duplicated lazy structure is for backwards compatibility; Qiskit has historically

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking care of this PR

TwoQubitWeylDecomposition,
)
6 changes: 5 additions & 1 deletion qiskit/synthesis/two_qubit/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,8 @@

"""Module containing two-qubit unitary synthesis methods."""

from .two_qubit_decompose import TwoQubitBasisDecomposer, two_qubit_cnot_decompose
from .two_qubit_decompose import (
TwoQubitBasisDecomposer,
two_qubit_cnot_decompose,
TwoQubitWeylDecomposition,
Comment on lines +17 to +18
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A search through qiskit-community showed that only these two objects are used. I'd suggest we use this as criterion and only add those classes to the API docs that are really needed 🙂

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine starting small, also looking at the code for all the specialized TwoQubitWeyl* variants it looks like they're all used internally by __new__ when you create a new TwoQubitWeylDecomposition object.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TwoQubitWeylDecomposer kind of implies that all the family classes are public in the sense of going to a lot of trouble to make a repr that you can eval back in (including pickling then b64'ing a Numpy array and making sure __new__ eats keywords that it throws in the repr just to be pretty) (it's a very busy constructor). That said, I don't really think it's necessary to make those public at all - they're largely not meant to be accessed directly, I think, because of the abstract factory class.

)
27 changes: 25 additions & 2 deletions qiskit/synthesis/two_qubit/two_qubit_decompose.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,26 @@ def decompose_two_qubit_product_gate(special_unitary_matrix):


class TwoQubitWeylDecomposition:
"""Decompose two-qubit unitary U = (K1l⊗K1r).Exp(i a xx + i b yy + i c zz).(K2l⊗K2r) , where U ∈
U(4), (K1l|K1r|K2l|K2r) ∈ SU(2), and we stay in the "Weyl Chamber" 𝜋/4 ≥ a ≥ b ≥ |c|
r"""Two-qubit Weyl decomposition.

Decompose two-qubit unitary

.. math::

U = ({K_1}^l \otimes {K_1}^r) \dot e^{(i a XX + i b YY + i c ZZ)} \dot ({K_2}^l \otimes {K_2}^r)

where

.. math::

U \in U(4),~
{K_1}^l, {K_1}^r, {K_2}^l, {K_2}^r \in SU(2)

and we stay in the "Weyl Chamber"

.. math::

\pi /4 \geq a \geq b \geq |c|

This is an abstract factory class that instantiates itself as specialized subclasses based on
the fidelity, such that the approximation error from specialization has an average gate fidelity
Expand All @@ -103,6 +121,11 @@ class TwoQubitWeylDecomposition:

Passing non-None fidelity to specializations is treated as an assertion, raising QiskitError if
forcing the specialization is more approximate than asserted.

Reference:
1. Cross, A. W., Bishop, L. S., Sheldon, S., Nation, P. D. & Gambetta, J. M.,
*Validating quantum computers using randomized model circuits*,
`arXiv:1811.12926 [quant-ph] <https://arxiv.org/abs/1811.12926>`_
"""

# The parameters of the decomposition:
Expand Down
Loading