From 5c9f223fae5874c490cc306ac6ac5e77d0b909cc Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Wed, 20 Sep 2023 17:01:06 -0400 Subject: [PATCH 1/3] Fix pickle handling for SingletonGate class This commit fixes an oversight in #10314 where the handling of pickle wasn't done correctly. Because the SingletonGate class defines __new__ and based on the parameters to the gate.__class__() call determines whether we get a new mutable copy or a shared singleton immutable instance we need special handling in pickle. By default pickle will call __new__() without any arguments and then rely on __setstate__ to update the state in the new object. This works fine if the original instance was a singleton but in the case of mutable copies this will create a singleton object instead of a mutable copy. To fix this a __reduce__ method is added to ensure arguments get passed to __new__ forcing a mutable object to be created in deserialization. Then a __setstate__ method is defined to correctly update the mutable object post creation. --- qiskit/circuit/singleton_gate.py | 19 ++++++++++++++++ test/python/circuit/test_singleton_gate.py | 25 ++++++++++++++++++++++ test/python/compiler/test_transpiler.py | 14 ++++++++++++ 3 files changed, 58 insertions(+) diff --git a/qiskit/circuit/singleton_gate.py b/qiskit/circuit/singleton_gate.py index 3d24247e31ca..b41da3541587 100644 --- a/qiskit/circuit/singleton_gate.py +++ b/qiskit/circuit/singleton_gate.py @@ -80,6 +80,25 @@ def __init__(self, *args, _condition=None, **kwargs): super().__init__(*args, **kwargs) self._condition = _condition + def __setstate__(self, state): + if state is not None: + self.label = state[0] + self._condition = state[1] + self.duration = state[2] + self.unit = state[3] + + def __reduce__(self): + if not self.mutable: + return (self.__class__, (), None) + # Argument list is set twice to force instantiation via __setstate__ + # the first arg list is only passed to __new__ to create a mutable + # object + return ( + self.__class__, + (self.label, self._condition, self.duration, self.unit), + (self.label, self._condition, self.duration, self.unit), + ) + def c_if(self, classical, val): if not isinstance(classical, (ClassicalRegister, Clbit)): raise CircuitError("c_if must be used with a classical register or classical bit") diff --git a/test/python/circuit/test_singleton_gate.py b/test/python/circuit/test_singleton_gate.py index d8c80661d675..b2a340dd4f02 100644 --- a/test/python/circuit/test_singleton_gate.py +++ b/test/python/circuit/test_singleton_gate.py @@ -18,6 +18,8 @@ """ import copy +import io +import pickle from qiskit.circuit.library import HGate, SXGate from qiskit.circuit import Clbit, QuantumCircuit, QuantumRegister, ClassicalRegister @@ -251,3 +253,26 @@ def test_positional_label(self): label_gate = SXGate("I am a little label") self.assertIsNot(gate, label_gate) self.assertEqual(label_gate.label, "I am a little label") + + def test_immutable_pickle(self): + gate = SXGate() + self.assertFalse(gate.mutable) + with io.BytesIO() as fd: + pickle.dump(gate, fd) + fd.seek(0) + copied = pickle.load(fd) + self.assertFalse(copied.mutable) + + def test_mutable_pickle(self): + gate = SXGate() + clbit = Clbit() + condition_gate = gate.c_if(clbit, 0) + self.assertIsNot(gate, condition_gate) + self.assertEqual(condition_gate.condition, (clbit, 0)) + self.assertTrue(condition_gate.mutable) + with io.BytesIO() as fd: + pickle.dump(condition_gate, fd) + fd.seek(0) + copied = pickle.load(fd) + self.assertEqual(copied, condition_gate) + self.assertTrue(copied.mutable) diff --git a/test/python/compiler/test_transpiler.py b/test/python/compiler/test_transpiler.py index 0be4922cb5b7..6d9555c15a49 100644 --- a/test/python/compiler/test_transpiler.py +++ b/test/python/compiler/test_transpiler.py @@ -2173,6 +2173,20 @@ def run(self, dag): added_cal = qc_test.calibrations["sx"][((0,), ())] self.assertEqual(added_cal, ref_cal) + @data(0, 1, 2, 3) + def test_parallel_singleton_conditional_gate(self, opt_level): + """Test that singleton mutable instance doesn't lose state in parallel.""" + backend = FakeNairobiV2() + circ = QuantumCircuit(2, 1) + circ.h(0) + circ.measure(0, circ.clbits[0]) + circ.z(1).c_if(circ.clbits[0], 1) + res = transpile( + [circ, circ], backend, optimization_level=opt_level, seed_transpiler=123456769 + ) + self.assertTrue(res[0].data[-1].operation.mutable) + self.assertEqual(res[0].data[-1].operation.condition, (res[0].clbits[0], 1)) + @data(0, 1, 2, 3) def test_backendv2_and_basis_gates(self, opt_level): """Test transpile() with BackendV2 and basis_gates set.""" From 3ad38b4ae95f23bae69d47942162345ed71ca6dd Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Fri, 22 Sep 2023 13:18:42 -0400 Subject: [PATCH 2/3] Use __getnewargs_ex__ insetad of __reduce__ & __setstate__ This commit pivots the pickle interface methods used to implement __getnewargs_ex__ instead of the combination of __reduce__ and __setstate__. Realistically, all we need to do here is pass that we have mutable arguments to new to trigger it to create a separate object, the rest of pickle was working correctly. This makes the interface being used for pickle a lot clearer. --- qiskit/circuit/singleton_gate.py | 20 +++----------------- 1 file changed, 3 insertions(+), 17 deletions(-) diff --git a/qiskit/circuit/singleton_gate.py b/qiskit/circuit/singleton_gate.py index b41da3541587..2fafa867b7f3 100644 --- a/qiskit/circuit/singleton_gate.py +++ b/qiskit/circuit/singleton_gate.py @@ -80,24 +80,10 @@ def __init__(self, *args, _condition=None, **kwargs): super().__init__(*args, **kwargs) self._condition = _condition - def __setstate__(self, state): - if state is not None: - self.label = state[0] - self._condition = state[1] - self.duration = state[2] - self.unit = state[3] - - def __reduce__(self): + def __getnewargs_ex__(self): if not self.mutable: - return (self.__class__, (), None) - # Argument list is set twice to force instantiation via __setstate__ - # the first arg list is only passed to __new__ to create a mutable - # object - return ( - self.__class__, - (self.label, self._condition, self.duration, self.unit), - (self.label, self._condition, self.duration, self.unit), - ) + return ((), {}) + return ((self.label, self._condition, self.duration, self.unit), {}) def c_if(self, classical, val): if not isinstance(classical, (ClassicalRegister, Clbit)): From c24edc1867b68a0c08c0ea5be3561e359387e3d3 Mon Sep 17 00:00:00 2001 From: Matthew Treinish Date: Fri, 22 Sep 2023 13:24:09 -0400 Subject: [PATCH 3/3] Improve assertion in immutable pickle test --- test/python/circuit/test_singleton_gate.py | 1 + 1 file changed, 1 insertion(+) diff --git a/test/python/circuit/test_singleton_gate.py b/test/python/circuit/test_singleton_gate.py index b2a340dd4f02..695216be450d 100644 --- a/test/python/circuit/test_singleton_gate.py +++ b/test/python/circuit/test_singleton_gate.py @@ -262,6 +262,7 @@ def test_immutable_pickle(self): fd.seek(0) copied = pickle.load(fd) self.assertFalse(copied.mutable) + self.assertIs(copied, gate) def test_mutable_pickle(self): gate = SXGate()