Quantum shannon decomposition failing for some inputs

[jsmallz333]

Environment

• Qiskit Terra version: 0.42.1
• Python version: 3.10.8

What is happening?

qs_decomposition in qsd.py throws an error for some examples, seemingly due to a bug in the code

How can we reproduce the issue?

from qiskit.quantum_info.synthesis.qsd import qs_decomposition
qs_decomposition(np.array([[0,1],[1,0]]))

Output:

Traceback (most recent call last):
  Cell In[14], line 3
    qs_decomposition(np.array([[0,1],[1,0]]))
  File /opt/conda/lib/python3.10/site-packages/qiskit/quantum_info/synthesis/qsd.py:122 in qs_decomposition
    return _apply_a2(circ)
  File /opt/conda/lib/python3.10/site-packages/qiskit/quantum_info/synthesis/qsd.py:253 in _apply_a2
    qc3 = two_qubit_decompose.two_qubit_cnot_decompose(mat2)
UnboundLocalError: local variable 'mat2' referenced before assignment

Use %tb to get the full traceback.

Alternatively,

qs_decomposition(qiskit.quantum_info.random_unitary(4).to_matrix())

Giving the same error

What should happen?

We should still be able to produce a decomposed circuit from these examples

Any suggestions?

This seems to occur when line 233 of qsd.py in the function ‘_apply_a2()’ does not transpile to include any of the ‘qsd2q’ gate type for an instance.

To fix this add something such as the following before the loop on line 242:

if not ind2q:
    return ccirc

Additionally should add a test for this kind of case to the test

[jsmallz333]

I can correct this with the above indicated as long as the reasoning follows

[jakelishman]

Yeah, I believe your reasoning is correct here, thanks - I don't think there's anything to do if there's nothing to decompose. @ewinston can check me, though, and I'll assign him to the PR if you're able to make it. Let us know if not, though, and one of us will.

[ewinston]

The _apply_a2 function actually wasn't supposed to by applied for dim == 2. I can submit a pr for that fix. Did you ever notice this for dimension > 2?

[jlapeyre]

Hi @jsmallz333 .

EDIT: I have added you as an author to the PR fixing this issue. The rest of this comment is now outdated.

We'd like to add you as an author to the PR since you suggested the correct fix. To add you manually, I'd need the email account associated with your github account. Alternatively, you can open a PR yourself, as I explain below.

In either case, you would need to sign a CLA. A notice including a link to do this will be added by a bot before the PR is merged.

Currently, we have the following: @ewinston opened https://github.com/Qiskit/qiskit-terra/pull/10126 to try to fix this. It doesn't really fix the problem, but is a closely related improvement, so I thought we should include it along with the proper fix. I then made this PR https://github.com/ewinston/qiskit-terra/pull/4 adding your suggested fix to his PR branch.

If you'd like to proceed as author, you can do whatever is most convenient, including one of the following:

• Put the email associated with @jsmallz333 in a comment here so that we can add you as author.
• Open a PR to fix this issue, and we'll close our existing PRs without merging them.
• Make a PR just like ewinston/qiskit-terra#4 , and I will close that one.