From 4e5d9f82bf4f5c4d5608e5f8ec37b9f3cf1c17c9 Mon Sep 17 00:00:00 2001 From: Julien Gacon Date: Tue, 26 Nov 2024 20:52:48 +0100 Subject: [PATCH] Fix parameter handling for `NLocal(..., flatten=True)` and standard gates (#13482) * Use _append_standard_gate directly * update params on cache * Revert "Use _append_standard_gate directly" This reverts commit 9769785c11385485ef2186d7aafa6d149f4d5fa3. * Add test and reno --- crates/circuit/src/circuit_data.rs | 16 +++++++++------- ...x-cached-params-update-4d2814b698fa76b4.yaml | 17 +++++++++++++++++ test/python/circuit/library/test_nlocal.py | 15 +++++++++++++++ test/python/circuit/test_parameters.py | 13 +++++++++++++ 4 files changed, 54 insertions(+), 7 deletions(-) create mode 100644 releasenotes/notes/fix-cached-params-update-4d2814b698fa76b4.yaml diff --git a/crates/circuit/src/circuit_data.rs b/crates/circuit/src/circuit_data.rs index 0025664e4d94..5422138264b3 100644 --- a/crates/circuit/src/circuit_data.rs +++ b/crates/circuit/src/circuit_data.rs @@ -1361,12 +1361,9 @@ impl CircuitData { let Param::ParameterExpression(expr) = ¶ms[parameter] else { return Err(inconsistent()); }; - params[parameter] = match bind_expr( - expr.bind_borrowed(py), - ¶m_ob, - value.as_ref(), - true, - )? { + let new_param = + bind_expr(expr.bind_borrowed(py), ¶m_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 '{}': '{}'", @@ -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())? } } diff --git a/releasenotes/notes/fix-cached-params-update-4d2814b698fa76b4.yaml b/releasenotes/notes/fix-cached-params-update-4d2814b698fa76b4.yaml new file mode 100644 index 000000000000..a4b2b6fe6571 --- /dev/null +++ b/releasenotes/notes/fix-cached-params-update-4d2814b698fa76b4.yaml @@ -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 `__. diff --git a/test/python/circuit/library/test_nlocal.py b/test/python/circuit/library/test_nlocal.py index 753feff490b1..22bfa391612d 100644 --- a/test/python/circuit/library/test_nlocal.py +++ b/test/python/circuit/library/test_nlocal.py @@ -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): diff --git a/test/python/circuit/test_parameters.py b/test/python/circuit/test_parameters.py index e291eb415813..2b342cae1968 100644 --- a/test/python/circuit/test_parameters.py +++ b/test/python/circuit/test_parameters.py @@ -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")