Qiskit / qiskit

Qiskit is an open-source SDK for working with quantum computers at the level of extended quantum circuits, operators, and primitives.
https://www.ibm.com/quantum/qiskit
Apache License 2.0
5.04k stars 2.32k forks source link

Circuit parameter binding does not bind `global_phase` in instruction definitions #10283

Closed jakelishman closed 1 year ago

jakelishman commented 1 year ago

Environment

What is happening?

When a custom gate with a set definition contains a parametrised global phase, any circuit that contains that custom gate will fail to bind the global phase during parameter assignment.

How can we reproduce the issue?

import math

from qiskit.circuit import Parameter, QuantumCircuit
from qiskit.quantum_info import Operator

x = Parameter("x")

custom = QuantumCircuit(1, global_phase=x).to_gate()
base = QuantumCircuit(1)
base.append(custom, [0], [])

Operator(base.assign_parameters({x: math.pi}))
TypeError                                 Traceback (most recent call last)
Cell In[1], line 12
      9 base = QuantumCircuit(1)
     10 base.append(custom, [0], [])
---> 12 Operator(base.assign_parameters({x: math.pi}))

File ~/code/qiskit/terra/qiskit/quantum_info/operators/operator.py:85, in Operator.__init__(self, data, input_dims, output_dims)
     76     self._data = np.asarray(data, dtype=complex)
     77 elif isinstance(data, (QuantumCircuit, Operation)):
     78     # If the input is a Terra QuantumCircuit or Operation we
     79     # perform a simulation to construct the unitary operator.
   (...)
     83     # conditional gates, measure, or reset will cause an
     84     # exception to be raised.
---> 85     self._data = self._init_instruction(data).data
     86 elif hasattr(data, "to_operator"):
     87     # If the data object has a 'to_operator' attribute this is given
     88     # higher preference than the 'to_matrix' method for initializing
     89     # an Operator object.
     90     data = data.to_operator()

File ~/code/qiskit/terra/qiskit/quantum_info/operators/operator.py:614, in Operator._init_instruction(cls, instruction)
    612 if isinstance(instruction, QuantumCircuit):
    613     instruction = instruction.to_instruction()
--> 614 op._append_instruction(instruction)
    615 return op

File ~/code/qiskit/terra/qiskit/quantum_info/operators/operator.py:691, in Operator._append_instruction(self, obj, qargs)
    689 else:
    690     new_qargs = [qargs[bit_indices[tup]] for tup in instruction.qubits]
--> 691 self._append_instruction(instruction.operation, qargs=new_qargs)

File ~/code/qiskit/terra/qiskit/quantum_info/operators/operator.py:669, in Operator._append_instruction(self, obj, qargs)
    666 if obj.definition.global_phase:
    667     dimension = 2**obj.num_qubits
    668     op = self.compose(
--> 669         ScalarOp(dimension, np.exp(1j * float(obj.definition.global_phase))),
    670         qargs=qargs,
    671     )
    672     self._data = op.data
    673 flat_instr = obj.definition

File ~/code/qiskit/terra/qiskit/circuit/parameterexpression.py:480, in ParameterExpression.__float__(self)
    478 except (TypeError, RuntimeError) as exc:
    479     if self.parameters:
--> 480         raise TypeError(
    481             "ParameterExpression with unbound parameters ({}) "
    482             "cannot be cast to a float.".format(self.parameters)
    483         ) from None
    484     try:
    485         # In symengine, if an expression was complex at any time, its type is likely to have
    486         # stayed "complex" even when the imaginary part symbolically (i.e. exactly)
    487         # cancelled out.  Sympy tends to more aggressively recognise these as symbolically
    488         # real.  This second attempt at a cast is a way of unifying the behaviour to the
    489         # more expected form for our users.
    490         cval = complex(self)

TypeError: ParameterExpression with unbound parameters ({Parameter(x)}) cannot be cast to a float.

What should happen?

All parameter usage in custom definitions should be bound.

Any suggestions?

This is due to ad-hoc recursion/iteration in QuantumCircuit._rebind_definition. It treats Instruction._definition like a list[tuple[Instruction, qubits, clbits]], which (a long time ago) was a possibility. Since Instruction._definition is now required to be a QuantumCircuit, the simplest fix is to make the definition-rebinding an explicit recursion via QuantumCircuit.assign_parameters.

muhundet commented 1 year ago

Hi @jakelishman, I don't think its a bug. In your code you are assigning parameters to the "base" Quantum Circuit instead of the "custom" Quantum Circuit. It is the "custom" Circuit that's expecting your global phase Parameter("x"). Operator(custom.assign_parameters({x: math.pi}))

Another thing is you need to remove the "to_gate()" method on your "custom" Circuit declaration so that you will be able to use "assign_parameter()" or "bind_parameter()" method.

Correct me if I am wrong or if it seem like I didn't get your issue right.

The code that I managed to run is below:

import math

from qiskit.circuit import Parameter, QuantumCircuit from qiskit.quantum_info import Operator

x = Parameter("x") custom = QuantumCircuit(1, global_phase=x) base = QuantumCircuit(1) base.append(custom, [0], [])

Operator(custom.assign_parameters({x: math.pi}))

The version of qiskit-terra installed on my machine is

{'qiskit-terra': '0.18.3', 'qiskit-aer': '0.9.1', 'qiskit-ignis': '0.6.0', 'qiskit-ibmq-provider': '0.17.0', 'qiskit-aqua': '0.9.5', 'qiskit': '0.31.0', 'qiskit-nature': None, 'qiskit-finance': None, 'qiskit-optimization': None, 'qiskit-machine-learning': None}

jakelishman commented 1 year ago

Hi there! Thanks for trying to help me - your code will actually run and it's how you'd make sure the global phase in an outer circuit is bound, you're correct. The trouble is that the code example I wrote should run as well; it's a normal part of Qiskit that you can define your own gates by calling to_gate() on a circuit you've made, and then put that into a larger circuit. We often want to do that when building circuits to expose the structure of the program we're making.

If some inner gates are parametrised, calling assign_parameters() on the outer circuit should recurse into the inner gates to parametrise (copies of) them too, so the whole circuit gets bound. It currently works if the parameters are in the gate calls of the definition of the inner gate, but it misses the global_phase (if any) of the custom gate's definition, which means there's still a parameter remaining, even after it should have been bound.

My example might look a bit useless for real code, and that's because what I've written here isn't code I was actively trying to use - it's a minimal reproducer for the bug. Real world code would most likely have other gate calls in the custom gate as well, and the base circuit would probably have lots of uses of custom inside it.

(While this certainly doesn't mean I can't be wrong, you also might not have known: I'm one of the maintainers/designers of Qiskit.)

muhundet commented 1 year ago

Thanx @jakelishman well understood.