Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix parameter handling for NLocal(..., flatten=True) and standard gates #13482

Merged
merged 4 commits into from
Nov 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 9 additions & 7 deletions crates/circuit/src/circuit_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1361,12 +1361,9 @@ impl CircuitData {
let Param::ParameterExpression(expr) = &params[parameter] else {
return Err(inconsistent());
};
params[parameter] = match bind_expr(
expr.bind_borrowed(py),
&param_ob,
value.as_ref(),
true,
)? {
let new_param =
bind_expr(expr.bind_borrowed(py), &param_ob, value.as_ref(), true)?;
params[parameter] = match new_param.clone_ref(py) {
Param::Obj(obj) => {
return Err(CircuitError::new_err(format!(
"bad type after binding for gate '{}': '{}'",
Expand All @@ -1382,8 +1379,13 @@ impl CircuitData {
#[cfg(feature = "cache_pygates")]
{
// Standard gates can all rebuild their definitions, so if the
// cached py_op exists, just clear out any existing cache.
// cached py_op exists, update the `params` attribute and clear out
// any existing cache.
if let Some(borrowed) = previous.py_op.get() {
borrowed
.bind(py)
.getattr(params_attr)?
.set_item(parameter, new_param)?;
borrowed.bind(py).setattr("_definition", py.None())?
}
}
Expand Down
17 changes: 17 additions & 0 deletions releasenotes/notes/fix-cached-params-update-4d2814b698fa76b4.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
---
fixes:
- |
Fixed a bug in :meth:`.QuantumCircuit.assign_parameters`, occurring when assigning parameters
to standard gates whose definition has already been triggered. In this case, the new values
were not properly propagated to the gate instances. While the circuit itself was still
compiled as expected, inspecting the individual operations would still show the old parameter.

For example::

from qiskit.circuit.library import EfficientSU2

circuit = EfficientSU2(2, flatten=True)
circuit.assign_parameters([1.25] * circuit.num_parameters, inplace=True)
print(circuit.data[0].operation.params) # would print θ[0] instead of 1.25

Fixed `#13478 <https://github.com/Qiskit/qiskit/issues/13478>`__.
15 changes: 15 additions & 0 deletions test/python/circuit/library/test_nlocal.py
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,21 @@ def test_initial_state_as_circuit_object(self):

self.assertCircuitEqual(ref, expected)

def test_inplace_assignment_with_cache(self):
"""Test parameters are correctly re-bound in the cached gates.

This test requires building with the Rust feature "cache_pygates" enabled, otherwise
it does not test what it is supposed to.

Regression test of #13478.
"""
qc = EfficientSU2(2, flatten=True)
binds = [1.25] * qc.num_parameters

qc.assign_parameters(binds, inplace=True)
bound_op = qc.data[0].operation
self.assertAlmostEqual(bound_op.params[0], binds[0])


@ddt
class TestNLocalFunction(QiskitTestCase):
Expand Down
13 changes: 13 additions & 0 deletions test/python/circuit/test_parameters.py
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,19 @@ def test_assign_parameters_by_iterable(self):
self.assertEqual(qc.assign_parameters(dict(zip(qc.parameters, binds)).values()), expected)
self.assertEqual(qc.assign_parameters(bind for bind in binds), expected)

def test_assign_parameters_with_cache(self):
"""Test assigning parameters on a circuit with already triggered cache."""
x = Parameter("x")
qc = QuantumCircuit(1)
qc.append(RZGate(x), [0]) # add via ``append`` to create a CircuitInstruction

_ = qc.data[0].operation.definition # trigger building the cache

binds = [1.2]
qc.assign_parameters(binds, inplace=True)

self.assertAlmostEqual(binds[0], qc.data[0].operation.params[0])

def test_bind_parameters_custom_definition_global_phase(self):
"""Test that a custom gate with a parametrized `global_phase` is assigned correctly."""
x = Parameter("x")
Expand Down