Skip to content

Commit

Permalink
Fix parameter handling for NLocal(..., flatten=True) and standard g…
Browse files Browse the repository at this point in the history
…ates (#13482)

* Use _append_standard_gate directly

* update params on cache

* Revert "Use _append_standard_gate directly"

This reverts commit 9769785.

* Add test and reno
  • Loading branch information
Cryoris authored Nov 26, 2024
1 parent 4c1de13 commit 4e5d9f8
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 7 deletions.
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

0 comments on commit 4e5d9f8

Please sign in to comment.