From 17a2ccc146305fdfeb709b6c86528dcd47110cc9 Mon Sep 17 00:00:00 2001 From: Raynel Sanchez <87539502+raynelfss@users.noreply.github.com> Date: Thu, 12 Dec 2024 03:22:05 -0500 Subject: [PATCH] Discard cache for standard gates in `assign_parameters` (#13557) * Fix: Discard cache when assigning new parameters in `CircuitData`. * Docs: Add release note * Test: Add test case * Docs: Add relevant comment --- crates/circuit/src/circuit_data.rs | 15 ++++++--------- .../fix-assign-parameters-ffa284ebde429704.yaml | 7 +++++++ test/python/transpiler/test_basis_translator.py | 17 +++++++++++++++++ 3 files changed, 30 insertions(+), 9 deletions(-) create mode 100644 releasenotes/notes/fix-assign-parameters-ffa284ebde429704.yaml diff --git a/crates/circuit/src/circuit_data.rs b/crates/circuit/src/circuit_data.rs index 1c12aef9efc7..fa36d8b841fb 100644 --- a/crates/circuit/src/circuit_data.rs +++ b/crates/circuit/src/circuit_data.rs @@ -1392,15 +1392,12 @@ impl CircuitData { #[cfg(feature = "cache_pygates")] { // Standard gates can all rebuild their definitions, so if the - // 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())? - } + // cached py_op exists, discard it to prompt the instruction + // to rebuild its cached python gate upon request later on. This is + // done to avoid an unintentional duplicated reference to the same gate + // instance in python. For more information, see + // https://github.com/Qiskit/qiskit/issues/13504 + previous.py_op.take(); } } else { // Track user operations we've seen so we can rebind their definitions. diff --git a/releasenotes/notes/fix-assign-parameters-ffa284ebde429704.yaml b/releasenotes/notes/fix-assign-parameters-ffa284ebde429704.yaml new file mode 100644 index 000000000000..b1e1dc5f67d6 --- /dev/null +++ b/releasenotes/notes/fix-assign-parameters-ffa284ebde429704.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Fix incorrect behavior in :class:`.CircuitData` in which, upon parameter assignment, + we attempted to modify the cached operation inside of a ``PackedInstruction``. Now + we instead discard said cache prompting the ``PackedInstruction`` to build a new Python + operation should it be needed. \ No newline at end of file diff --git a/test/python/transpiler/test_basis_translator.py b/test/python/transpiler/test_basis_translator.py index 820c4f241a1e..06762a78f1fa 100644 --- a/test/python/transpiler/test_basis_translator.py +++ b/test/python/transpiler/test_basis_translator.py @@ -21,6 +21,7 @@ from qiskit import QuantumRegister, ClassicalRegister, QuantumCircuit from qiskit import transpile from qiskit.circuit import Gate, Parameter, EquivalenceLibrary, Qubit, Clbit, Measure +from qiskit.circuit.equivalence_library import StandardEquivalenceLibrary as std_eq_lib from qiskit.circuit.classical import expr, types from qiskit.circuit.library import ( HGate, @@ -481,6 +482,22 @@ def test_different_bits(self): out = BasisTranslator(std_eqlib, basis).run(circuit_to_dag(base)) self.assertEqual(set(out.count_ops(recurse=True)), basis) + def test_correct_parameter_assignment(self): + """Test correct parameter assignment from an equivalence during translation""" + rx_key = next(key for key in std_eq_lib.keys() if key.name == "rx") + + # The circuit doesn't need to be parametric. + qc = QuantumCircuit(1) + qc.rx(0.5, 0) + + BasisTranslator( + equivalence_library=std_eq_lib, + target_basis=["cx", "id", "rz", "sx", "x"], + )(qc) + + inst = std_eq_lib._get_equivalences(rx_key)[0].circuit.data[0] + self.assertEqual(inst.params, inst.operation.params) + class TestUnrollerCompatability(QiskitTestCase): """Tests backward compatability with the Unroller pass.