Instruction.params updates not reflected in definition (if definition already called)

[kdk]

Information

• Qiskit Terra version: master
• Python version: 3.5
• Operating system: osx 10.14

What is the current behavior?

.params values are fixed in gate definitions the first time ._define is called, and not updated, even if Instruction.params changes.

>>> from qiskit.extensions.standard import RZGate
>>> # Create a gate, and check the parameter values in its definition
...
>>> g = RZGate(0.1)
>>> g.definition
[(<qiskit.extensions.standard.u1.U1Gate object at 0x10b181208>, [Qubit(QuantumRegister(1, 'q'), 0)], [])]
>>> g.definition[0][0].params
[0.1]
>>> # Create a gate, change its parameters, then check parameter values in its definition
...
>>> g = RZGate(0.1)
>>> g.params[0] = 0.2
>>> g.definition[0][0].params
[0.2]
>>> # Create a gate, check its definition, updates its params, and see if the update is reflected in the definition
...
>>> g = RZGate(0.1)
>>> g.definition[0][0].params
[0.1]
>>> g.params[0] = 0.2
>>> g.definition[0][0].params
[0.1]

[kdk]

A possible solution would be to, at the end of the Instruction.params setter, call self._define (or reset self._definition to None).

[1ucian0]

priority: medium because bug is obscure and might hide misbehavior. Reassess if disagree.

[willhbang]

I can grab this one if no one's started. Looks like the fix and test cases have already been spelled out.

I had a question though, because I'm a python newbie: The case in the issue description, g.params[0] = 0.2, bypasses the params setter invocation, so resetting self._definition there wouldn't work. Is mutating instance attributes like that a common pattern that we have to anticipate? In contrast, I can see why g.params = [0.2], which does invoke the setter, is a case we should handle (and does get fixed by resetting self._definition).

[kdk]

The case in the issue description, g.params[0] = 0.2, bypasses the params setter invocation, so resetting self._definition there wouldn't work. Is mutating instance attributes like that a common pattern that we have to anticipate?

In general, yes and you're right that this wouldn't trigger the params setter. It is possible to work around this (e.g. what's done with QuantumCircuit.data and the QuantumCircuitData class in #2838 ), but it's not necessarily trivial.

[willhbang]

Oh interesting, so this is a recurring issue in a way. That is, we need to execute additional logic when updating certain properties of a circuit component (or the circuit itself, in the case of QuantumCircuit.data), while maintaining the list interface of that property. Is it worth generalizing that? E.g. have a new class like ComponentData that sits between QuantumCircuit.data/Instruction.params and MutableSequence, to manage those properties.

Maybe that's overkill though, we could also just have a class InstructionParams that directly manages Instruction.params.

[jwoehr]

Still valid. Here's a cleanup of the example:

from qiskit.circuit.library import RZGate
print('Create a gate, and check the parameter values in its definition')
g = RZGate(0.1)
print(g.definition)
print(g.definition[0][0].params)
print('Create a gate, change its parameters, then check parameter values in its definition')
g = RZGate(0.1)
g.params[0] = 0.2
print(g.definition[0][0].params)
print('Create a gate, check its definition, updates its params, and see if the update is reflected in the definition')
g = RZGate(0.1)
print(g.definition[0][0].params)
g.params[0] = 0.2
print(g.definition[0][0].params)

[Cryoris]

Personally, I'm not convinced params should be publically writeable at all. It might be both safer to users and easier for us to handle if gates define setters for mutable attributes. Parameterized gates, for instance, can be bound (or re-bound in future?) to certain values but covering the bind_parameters path and a direct access seems overcomplicated. ]