Split2QUnitaries assumes node.op is ndarray

[qci-amos]

The TwoQubitWeylDecomposition says it only accepts ndarray:

qiskit/qiskit/synthesis/two_qubit/two_qubit_decompose.py

Lines 184 to 190 in 713ab38

	def __init__(
	self,
	unitary_matrix: np.ndarray,
	fidelity: float | None = 1.0 - 1.0e-9,
	*,
	_specialization: two_qubit_decompose.Specialization | None = None,
	):

However this line is passing a node op:

qiskit/qiskit/transpiler/passes/optimization/split_2q_unitaries.py

Line 53 in 713ab38

decomp = TwoQubitWeylDecomposition(node.op, fidelity=self.requested_fidelity)

some of my code is now failing in qiskit 1.2.0 because in some of my unit tests, this is an Instruction which is, for what it's worth, not a gate in the qiskit library.

My problem goes away if I replace this:

    circuit = qiskit.transpile(circuit, basis_gates=basis_gates, optimization_level=2)

in my test with:

    circuit = qiskit.transpile(circuit, basis_gates=basis_gates, optimization_level=1)

Linking associated PRs:
#12727
#12898

[mtreinish]

Do you have example code and a traceback showing the error you're hitting? The object model/type should be protecting against this if your custom class is an Instruction subclass and not a Gate subclass I would expect the condition attempting to skip attempting the decomposition to always be triggered because the node.matrix will always be None for an Instruction subclass that's not a Gate subclass.

Looking at the code you're right there is a mismatch with the argument typing and the pass is not accounting for the case it should be passing node.matrix it's implicitly relying on gates with matrices also implementing __array__ which is not a requirement of Gate subclasses. This will be fixed in 1.2.1, and should just call node.matrix instead of node.op as the input to the TwoQubitWeylDecomposition, but it would be good to confirm that this is actually what's going on in your local code because if you're running with a subclass of Instruction directly I think we need to do some more digging.

[jakelishman]

Thanks for the report. Do you have a small producer script that shows the problem?

I think I know what's happening, but it'd be good to be certain. The type hint of the decomposer isn't really correct, and it's actually anything that can be cast to an array, which is true of most gates - but there's no requirement that a user gate has a matrix form. (Fwiw, if you want to, give your gate a __array__ method.)

[qci-amos]

This reproduces the problem:

import numpy as np
import qiskit
from qiskit import QuantumCircuit
from qiskit.circuit import Gate, QuantumCircuit


class MyGate(Gate):
    def __init__(self):
        super().__init__("mygate", 2, [])

    def to_matrix(self):
        return np.eye(4, dtype=complex)
        # return np.eye(4, dtype=float)


def mygate(self, qubit1, qubit2):
    return self.append(MyGate(), [qubit1, qubit2], [])


QuantumCircuit.mygate = mygate

qc = QuantumCircuit(2)
qc.mygate(0, 1)

basis_gates = [
    "mygate",
]

circuit = qiskit.transpile(qc, basis_gates=basis_gates, optimization_level=2)

gives:

  File "/home/amos/miniconda3/envs/py311/lib/python3.11/site-packages/qiskit/transpiler/passes/optimization/split_2q_unitaries.py", line 54, in run
    decomp = TwoQubitWeylDecomposition(node.op, fidelity=self.requested_fidelity)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/amos/miniconda3/envs/py311/lib/python3.11/site-packages/qiskit/synthesis/two_qubit/two_qubit_decompose.py", line 191, in __init__
    unitary_matrix = np.asarray(unitary_matrix, dtype=complex)
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: must be real number, not MyGate

Note: the dtype of my actual matrix needs to be complex. If I use float instead in this example there's no exception.

[jakelishman]

Thanks! As an immediate workaround, you should rename your to_matrix method to __array__: the Gate.to_matrix method isn't fully meant to be virtual; it uses an existing __array__, which you can see in the docs here: https://docs.quantum.ibm.com/api/qiskit/circuit#creating-instruction-subclasses (though I'll admit, I don't love that split).

That said, there's still no reason for the code to crash on this example, and we can get Split2qUnitaries fixed.

[qci-amos]

If I try this:

    # def to_matrix(self):
    def __array__(self, dtype=None, copy=None):
        return np.eye(4, dtype=complex)
        # return np.eye(4, dtype=float)

if that's approximately what you meant, then I get a different exception:

qiskit.transpiler.exceptions.TranspilerError: "Unable to translate the operations in the circuit: ['u'] to the backend's (or manually specified) target basis: ['while_loop', 'delay', 'reset', 'mygate', 'if_else', 'for_loop', 'snapshot', 'barrier', 'switch_case', 'measure', 'store']. This likely means the target basis is not universal or there are additional equivalence rules needed in the EquivalenceLibrary being used. For more details on this error see: https://docs.quantum.ibm.com/api/qiskit/qiskit.transpiler.passes.BasisTranslator#translation-errors"

Probably that's just due to how crude my example is. I'm ok with my workaround of lowering the optimization level because this is just old test code I've been lugging around and should probably just delete.

Thanks for the link -- that page didn't exist years ago when I wrote these gates! 😁

[jakelishman]

Thanks - out of curiosity, would transpile(qc, basis_gates=basis_gates, optimization_level=2) work with your setup (with the __array__ method defined) in Qiskit 1.1? We had originally merged Split2qUnitaries thinking that it wouldn't cause transpiler failures that wouldn't already have seen a transpiler failure later, but if we made a mistake there, we need to rethink.

(About the subclassing docs: yeah, they just got written very recently. A bit embarrassing that it took us as long as this, really...)

[qci-amos]

Yes, the specific unit test would pass if if my original code I switched:

-    def to_matrix(self):
+    def __array__(self, *args, **kwargs):

[jakelishman]

Ok, if it previously passed at O2 and now doesn't, then it feels like we've made a mistake in the preset pass-manager logic about when it's acceptable to run Split2QUnitaries, in addition to the actual code bug caused by running it right now. Thanks for the fast responses - we'll hopefully get both components of this fixed in a 1.2.1 patch release within a couple of weeks.

[ElePT]

I think that this transpiler exception comes from the change in the internal transpile logic to build a target from loose transpiler constraints (#12185), not from the use of Split2QUnitaries. The snippet from @qci-amos provides a list of basis gates that only contains the custom gate ("mygate"), so the pass manager generation code builds a target that doesn't contain any other basis gate (just the instructions added by default to all targets). This incomplete basis set is what causes BasisTranslator to fail and raise the mentioned error.

You can fix the example by simply completing the basis set:

class MyGate(Gate):
    def __init__(self):
        super().__init__("mygate", 2, [])

    def __array__(self, *args, **kwargs):
        return np.eye(4, dtype=complex)
        # return np.eye(4, dtype=float)

def mygate(self, qubit1, qubit2):
    return self.append(MyGate(), [qubit1, qubit2], [])


QuantumCircuit.mygate = mygate

qc = QuantumCircuit(2)
qc.mygate(0, 1)

basis_gates = [
    "mygate","cx", "x", "sx", "id", "rz"
]

circuit = transpile(qc, basis_gates=basis_gates, optimization_level=2)

This was not the case in 1.1 because the passes were run differently in the cases where a target wasn't explicitly provided (and now all passes follow the "target path"), but this behavior wasn't super clearly documented. I will think of what we can do to better document edge cases like this.

[jakelishman]

Elena: for the input circuit, the basis ["my_gate"] is complete, so I think we need to ensure that the paths don't cause a breakage like this - it shouldn't be required to include additional gates to facilitate unnecessary resynthesis.

[Cryoris]

To add some more details, this does indeed break because Split2QUnitaries splits the 2q MyGate into two 1q U instructions -- which are not part of the basis gate set. Maybe Split2QUnitaries should not be run on gates that are already in the basis?

# Running the script with optimization level 2....
BasisTranslator got:
   ┌─────────────────┐
q: ┤ U3(π/2,phi,lam) ├
   └─────────────────┘
BasisTranslator got:
     ┌─────────┐
q_0: ┤0        ├
     │  Mygate │
q_1: ┤1        ├
     └─────────┘
Pre Split2QUnitaries:
     ┌─────────┐
q_0: ┤0        ├
     │  Mygate │
q_1: ┤1        ├
     └─────────┘
Post Split2QUnitaries:
     ┌─────────┐
q_0: ┤ Unitary ├
     ├─────────┤
q_1: ┤ Unitary ├
     └─────────┘

BasisTranslator got:
     ┌──────────┐
q_0: ┤ U(0,0,0) ├
     ├──────────┤
q_1: ┤ U(0,0,0) ├
     └──────────┘
# code breaks now: unsupported basis

[mtreinish]

That's an odd edge case to consider. because it means there is a 2q basis gate that is the product of single qubit gates. I'm not sure what the correct/expected behavior is in that case. My gut is that we should always be breaking up gates like this, but I never considered a target that included such a gate.

[qci-amos]

FWIW, in my real code, I have several gates like this one here that are actually 2q.

[Cryoris]

So currently we run Split2QUnitaries, assuming that we have a complete 1-qubit basis set, which is not always given. For example, we also run into the same issue if the gate could be decomposed into our basis, but Split2QUnitaries interferes:

from qiskit import QuantumCircuit, transpile
from qiskit.circuit.library import *

gate = PauliGate("XX")

qc = QuantumCircuit(2)
qc.append(gate, [0, 1])

basis_gates = ["x"]  # gate decomposes into XX
 
circuit = transpile(qc, basis_gates=basis_gates, optimization_level=1)  # works
circuit = transpile(qc, basis_gates=basis_gates, optimization_level=2)  # fails -- cannot map U to the basis

So maybe we should disable Split2QUnitaries in transpile if we don't have a complete basis set. To check if we have a complete basis set we could check whether some specific gates are in the basis gates, or we could try to decompose the output of the pass (probably less nice, as expensive).

[Cryoris]

#13015 with #13095 should fix these issues so I'll close this -- feel free to re-open if we missed anything!