ControlFlowOp with parameterized body is not parameterized

[nkanazawa1989]

What should we add?

When I test parameterization of below control flow, I would expect it is parameterized.

from qiskit.circuit import QuantumCircuit, Qubit, Clbit, Parameter

p1 = Parameter("P1")
p2 = Parameter("P2")

bits = [Qubit(), Qubit(), Clbit()]
qc = QuantumCircuit(bits)
qc.sx(0)
qc.measure(0, 0)
with qc.if_test((bits[2], 0)) as else_:
    qc.rx(p1, 0)
with else_:
    qc.rx(p2, 0)

qc._data[2].operation.is_parameterized()

False

But I get False because is_parameterized function is defined in Instruction and it only tests against ParameterExpression type. ControlFlowOp must overwrite the method to recursively check parameterization of its body.

I'm not sure if this is expected behavior. If we change current behavior this is indeed an API break and we should be very careful.

[jakelishman]

I'm not sure what, if any, the intended behaviour was, really - I had forgotten this method even existed.

I'm not 100% certain what the best path is here. I very roughly lean towards leaving it as-is and documenting the behaviour clearly, because as you say, it'd be a breaking change, and the problem has been around ever since control flow was first introduced. That said, I forgot the method even existed, so I clearly don't really know how it's being used. If there's a clear compelling use case for making it have the new behaviour, we can go that way.

[nkanazawa1989]

I'm fine with only updating documentation (or maybe deprecate and overwrite with NotImplemented). Sometime this can be useful for payload validation, e.g. checking if all unbound parameters are operands of allowed gate type. However, this method is just a syntactic sugar of

inst.operation.is_parameterized() == any(isinstance(p, ParameterExpression) for p in inst.operation.params)

and I don't think the readability of the code is improved so much. But if we write validation logic with this method, the method is called very frequently, and if we implement this with Rust, probably we can get some performance gain.

[AlbertJP]

The same problem happens with ControlledGate objects: they are also not parameterized, even if their base_gate is - because the variable _params, on which Instruction.is_parameterized() relies, is not set in the setter of ControlledGate.params.

This leads to problems when using gradient estimators from Qiskit Algorithms on circuits with controlled parameterised gates: the check in https://github.com/qiskit-community/qiskit-algorithms/blob/7f598a8fbf9ca0adb5a4521af5acad9a7137ef5d/qiskit_algorithms/gradients/utils.py#L302 unexpectedly returns false.