From 39ee348464f9b8f6350678d96b15c4af6eed20e0 Mon Sep 17 00:00:00 2001 From: Cryoris Date: Mon, 2 Mar 2020 10:31:23 +0100 Subject: [PATCH 1/9] parameters are insertion ordered --- qiskit/circuit/quantumcircuit.py | 6 +- qiskit/compiler/assemble.py | 2 +- qiskit/converters/circuit_to_gate.py | 4 +- qiskit/converters/circuit_to_instruction.py | 4 +- test/python/circuit/test_parameters.py | 68 +++++++++++++------ .../converters/test_circuit_to_instruction.py | 2 +- 6 files changed, 58 insertions(+), 28 deletions(-) diff --git a/qiskit/circuit/quantumcircuit.py b/qiskit/circuit/quantumcircuit.py index 461a27dd9466..48ce7bdcf4be 100644 --- a/qiskit/circuit/quantumcircuit.py +++ b/qiskit/circuit/quantumcircuit.py @@ -1238,7 +1238,9 @@ def from_qasm_str(qasm_str): @property def parameters(self): """Convenience function to get the parameters defined in the parameter table.""" - return set(self._parameter_table.keys()) + if len(set(self._parameter_table.keys())) != len(list(self._parameter_table.keys())): + raise ValueError('Some parameters existed twice!') + return list(self._parameter_table.keys()) def bind_parameters(self, value_dict): """Assign parameters to values yielding a new circuit. @@ -1255,7 +1257,7 @@ def bind_parameters(self, value_dict): new_circuit = self.copy() unrolled_value_dict = self._unroll_param_dict(value_dict) - if unrolled_value_dict.keys() > self.parameters: + if unrolled_value_dict.keys() > set(self.parameters): raise CircuitError('Cannot bind parameters ({}) not present in the circuit.'.format( [str(p) for p in value_dict.keys() - self.parameters])) diff --git a/qiskit/compiler/assemble.py b/qiskit/compiler/assemble.py index cf4866228b5d..819ca380f890 100644 --- a/qiskit/compiler/assemble.py +++ b/qiskit/compiler/assemble.py @@ -326,7 +326,7 @@ def _expand_parameters(circuits, run_config): all_bind_parameters = [bind.keys() for bind in parameter_binds] - all_circuit_parameters = [circuit.parameters for circuit in circuits] + all_circuit_parameters = [set(circuit.parameters) for circuit in circuits] # Collect set of all unique parameters across all circuits and binds unique_parameters = {param diff --git a/qiskit/converters/circuit_to_gate.py b/qiskit/converters/circuit_to_gate.py index a313b17a8149..264dc011ea76 100644 --- a/qiskit/converters/circuit_to_gate.py +++ b/qiskit/converters/circuit_to_gate.py @@ -56,14 +56,14 @@ def circuit_to_gate(circuit, parameter_map=None): else: parameter_dict = circuit._unroll_param_dict(parameter_map) - if parameter_dict.keys() != circuit.parameters: + if parameter_dict.keys() != set(circuit.parameters): raise QiskitError(('parameter_map should map all circuit parameters. ' 'Circuit parameters: {}, parameter_map: {}').format( circuit.parameters, parameter_dict)) gate = Gate(name=circuit.name, num_qubits=sum([qreg.size for qreg in circuit.qregs]), - params=sorted(parameter_dict.values(), key=lambda p: p.name)) + params=list(parameter_dict.values())) gate.condition = None def find_bit_position(bit): diff --git a/qiskit/converters/circuit_to_instruction.py b/qiskit/converters/circuit_to_instruction.py index 6001c9de659a..b50895231ff7 100644 --- a/qiskit/converters/circuit_to_instruction.py +++ b/qiskit/converters/circuit_to_instruction.py @@ -64,7 +64,7 @@ def circuit_to_instruction(circuit, parameter_map=None): else: parameter_dict = circuit._unroll_param_dict(parameter_map) - if parameter_dict.keys() != circuit.parameters: + if parameter_dict.keys() != set(circuit.parameters): raise QiskitError(('parameter_map should map all circuit parameters. ' 'Circuit parameters: {}, parameter_map: {}').format( circuit.parameters, parameter_dict)) @@ -72,7 +72,7 @@ def circuit_to_instruction(circuit, parameter_map=None): instruction = Instruction(name=circuit.name, num_qubits=sum([qreg.size for qreg in circuit.qregs]), num_clbits=sum([creg.size for creg in circuit.cregs]), - params=sorted(parameter_dict.values(), key=lambda p: p.name)) + params=list(parameter_dict.values())) instruction.condition = None def find_bit_position(bit): diff --git a/test/python/circuit/test_parameters.py b/test/python/circuit/test_parameters.py index d940c16a4245..c819a7896d6f 100644 --- a/test/python/circuit/test_parameters.py +++ b/test/python/circuit/test_parameters.py @@ -95,6 +95,34 @@ def test_fix_variable(self): self.assertEqual(float(bqc.data[0][0].params[0]), 0.6) self.assertEqual(float(bqc.data[1][0].params[1]), 0.6) + @data('circuit', 'gate', 'instruction', 'nested') + def test_sorted_by_insertion(self, target): + """Test that parameters are sorted by insertion.""" + params = [Parameter('2'), Parameter('13'), Parameter('0')] + qc = QuantumCircuit(2) + qc.rx(params[0], 0) + qc.rx(params[1], 1) + qc.rx(params[2], 0) + + if target == 'circuit': + self.assertEqual(qc.parameters, params) + elif target == 'gate': + self.assertEqual(qc.to_gate().params, params) + elif target == 'instruction': + self.assertEqual(qc.to_gate().params, params) + elif target == 'nested': + gate = qc.to_gate() + qc2 = QuantumCircuit(2) + before, after = Parameter('before'), Parameter('after') + qc2.rz(before, 1) + qc2.append(gate, [0, 1]) + qc2.rz(after, 0) + + self.assertEqual(qc2.parameters, [before] + params + [after]) + else: + raise ValueError('Unsupported target {}'.format(target)) + + def test_multiple_parameters(self): """Test setting multiple parameters""" theta = Parameter('θ') @@ -103,7 +131,7 @@ def test_multiple_parameters(self): qc = QuantumCircuit(qr) qc.rx(theta, qr) qc.u3(0, theta, x, qr) - self.assertEqual(qc.parameters, {theta, x}) + self.assertEqual(qc.parameters, [theta, x]) def test_partial_binding(self): """Test that binding a subset of circuit parameters returns a new parameterized circuit.""" @@ -116,7 +144,7 @@ def test_partial_binding(self): pqc = qc.bind_parameters({theta: 2}) - self.assertEqual(pqc.parameters, {x}) + self.assertEqual(pqc.parameters, [x]) self.assertEqual(float(pqc.data[0][0].params[0]), 2) self.assertEqual(float(pqc.data[1][0].params[1]), 2) @@ -133,14 +161,14 @@ def test_expression_partial_binding(self): pqc = qc.bind_parameters({theta: 2}) - self.assertEqual(pqc.parameters, {phi}) + self.assertEqual(pqc.parameters, [phi]) self.assertTrue(isinstance(pqc.data[0][0].params[0], ParameterExpression)) self.assertEqual(str(pqc.data[0][0].params[0]), 'phi + 2') fbqc = pqc.bind_parameters({phi: 1}) - self.assertEqual(fbqc.parameters, set()) + self.assertEqual(fbqc.parameters, []) self.assertTrue(isinstance(fbqc.data[0][0].params[0], ParameterExpression)) self.assertEqual(float(fbqc.data[0][0].params[0]), 3) @@ -156,14 +184,14 @@ def test_expression_partial_binding_zero(self): pqc = qc.bind_parameters({theta: 0}) - self.assertEqual(pqc.parameters, {phi}) + self.assertEqual(pqc.parameters, [phi]) self.assertTrue(isinstance(pqc.data[0][0].params[0], ParameterExpression)) self.assertEqual(str(pqc.data[0][0].params[0]), '0') fbqc = pqc.bind_parameters({phi: 1}) - self.assertEqual(fbqc.parameters, set()) + self.assertEqual(fbqc.parameters, []) self.assertTrue(isinstance(fbqc.data[0][0].params[0], ParameterExpression)) self.assertEqual(float(fbqc.data[0][0].params[0]), 0) @@ -227,7 +255,7 @@ def test_circuit_composition(self): qc2.measure(qr, cr) qc3 = qc1 + qc2 - self.assertEqual(qc3.parameters, {theta, phi}) + self.assertEqual(qc3.parameters, [theta, phi]) def test_composite_instruction(self): """Test preservation of parameters via parameterized instructions.""" @@ -246,7 +274,7 @@ def test_composite_instruction(self): qc2.ry(phi, qr2[0]) qc2.h(qr2) qc2.append(gate, qargs=[qr2[1]]) - self.assertEqual(qc2.parameters, {theta, phi}) + self.assertEqual(qc2.parameters, [phi, theta]) def test_parameter_name_conflicts_raises(self): """Verify attempting to add different parameters with matching names raises an error.""" @@ -456,15 +484,15 @@ def test_decompose_propagates_bound_parameters(self, target_type): bound_qc2 = qc2.bind_parameters({theta: 0.5}) - self.assertEqual(qc2.parameters, {theta}) - self.assertEqual(bound_qc2.parameters, set()) + self.assertEqual(qc2.parameters, [theta]) + self.assertEqual(bound_qc2.parameters, []) decomposed_qc2 = bound_qc2.decompose() expected_qc2 = QuantumCircuit(1) expected_qc2.rx(0.5, 0) - self.assertEqual(decomposed_qc2.parameters, set()) + self.assertEqual(decomposed_qc2.parameters, []) self.assertEqual(decomposed_qc2, expected_qc2) @data('gate', 'instruction') @@ -492,8 +520,8 @@ def test_decompose_propagates_deeply_bound_parameters(self, target_type): bound_qc3 = qc3.bind_parameters({theta: 0.5}) - self.assertEqual(qc3.parameters, {theta}) - self.assertEqual(bound_qc3.parameters, set()) + self.assertEqual(qc3.parameters, [theta]) + self.assertEqual(bound_qc3.parameters, []) decomposed_qc3 = bound_qc3.decompose() deep_decomposed_qc3 = decomposed_qc3.decompose() @@ -501,7 +529,7 @@ def test_decompose_propagates_deeply_bound_parameters(self, target_type): expected_qc3 = QuantumCircuit(1) expected_qc3.rx(0.5, 0) - self.assertEqual(deep_decomposed_qc3.parameters, set()) + self.assertEqual(deep_decomposed_qc3.parameters, []) self.assertEqual(deep_decomposed_qc3, expected_qc3) @data('gate', 'instruction') @@ -751,7 +779,7 @@ def test_to_instruction_with_expression(self, target_type, order): elif target_type == 'instruction': gate = qc1.to_instruction() - self.assertEqual(gate.params, [phi, theta]) + self.assertEqual(gate.params, [theta, phi]) delta = Parameter('delta') qr2 = QuantumRegister(3, name='qr2') @@ -759,7 +787,7 @@ def test_to_instruction_with_expression(self, target_type, order): qc2.ry(delta, qr2[0]) qc2.append(gate, qargs=[qr2[1]]) - self.assertEqual(qc2.parameters, {delta, theta, phi}) + self.assertEqual(qc2.parameters, [delta, theta, phi]) binds = {delta: 1, theta: 2, phi: 3} expected_qc = QuantumCircuit(qr2) @@ -773,7 +801,7 @@ def test_to_instruction_with_expression(self, target_type, order): elif order == 'decompose-bind': decomp_bound_qc = qc2.decompose().bind_parameters(binds) - self.assertEqual(decomp_bound_qc.parameters, set()) + self.assertEqual(decomp_bound_qc.parameters, []) self.assertEqual(decomp_bound_qc, expected_qc) @combine(target_type=['gate', 'instruction'], @@ -798,7 +826,7 @@ def test_to_instruction_expression_parameter_map(self, target_type, order): elif target_type == 'instruction': gate = qc1.to_instruction(parameter_map={theta: theta_p, phi: phi_p}) - self.assertEqual(gate.params, [phi_p, theta_p]) + self.assertEqual(gate.params, [theta_p, phi_p]) delta = Parameter('delta') qr2 = QuantumRegister(3, name='qr2') @@ -806,7 +834,7 @@ def test_to_instruction_expression_parameter_map(self, target_type, order): qc2.ry(delta, qr2[0]) qc2.append(gate, qargs=[qr2[1]]) - self.assertEqual(qc2.parameters, {delta, theta_p, phi_p}) + self.assertEqual(qc2.parameters, [delta, theta_p, phi_p]) binds = {delta: 1, theta_p: 2, phi_p: 3} expected_qc = QuantumCircuit(qr2) @@ -820,7 +848,7 @@ def test_to_instruction_expression_parameter_map(self, target_type, order): elif order == 'decompose-bind': decomp_bound_qc = qc2.decompose().bind_parameters(binds) - self.assertEqual(decomp_bound_qc.parameters, set()) + self.assertEqual(decomp_bound_qc.parameters, []) self.assertEqual(decomp_bound_qc, expected_qc) def test_binding_across_broadcast_instruction(self): diff --git a/test/python/converters/test_circuit_to_instruction.py b/test/python/converters/test_circuit_to_instruction.py index 1342eab044c4..339f80894d08 100644 --- a/test/python/converters/test_circuit_to_instruction.py +++ b/test/python/converters/test_circuit_to_instruction.py @@ -61,7 +61,7 @@ def test_flatten_parameters(self): inst = circuit_to_instruction(qc) - self.assertEqual(inst.params, [phi, theta]) + self.assertEqual(inst.params, [theta, phi]) self.assertEqual(inst.definition[0][0].params, [theta]) self.assertEqual(inst.definition[1][0].params, [phi]) self.assertEqual(inst.definition[2][0].params, [theta, phi]) From c42f4fd4e5829f01e00720b9e95972a740ace78a Mon Sep 17 00:00:00 2001 From: Cryoris Date: Mon, 2 Mar 2020 10:37:14 +0100 Subject: [PATCH 2/9] add reno --- ...ion-ordered-parameters-e8a4a44c3912df6f.yaml | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 releasenotes/notes/insertion-ordered-parameters-e8a4a44c3912df6f.yaml diff --git a/releasenotes/notes/insertion-ordered-parameters-e8a4a44c3912df6f.yaml b/releasenotes/notes/insertion-ordered-parameters-e8a4a44c3912df6f.yaml new file mode 100644 index 000000000000..df10b57141d4 --- /dev/null +++ b/releasenotes/notes/insertion-ordered-parameters-e8a4a44c3912df6f.yaml @@ -0,0 +1,17 @@ +--- +features: + - | + Since Python 3.6 dictionaries are insertion ordered, i.e. retrieving the keys of a + dictionary will return an iterable mirroring the order in which the keys are inserted. + In the QuantumCircuit, Parameters are stored in a dictionary so the information of + the insertion order is present. + Currently, we discard this information at two points: + 1) if ``QuantumCircuit.parameters`` is called we return a set of the parameters, + which can change the order + 2) if we create a ``Gate`` or ``Instruction`` out of a circuit, the parameters are + actively sorted by name + The new behaviour ensures that parameters are insertion-sorted by + 1) returning a list, not set, of the parameter dictionary keys + 2) not re-sorting the parameters of the circuit + This feature mirrors the behaviour some users might expect and is necessary for changes + in Aqua introduced by the Ansatz object. \ No newline at end of file From 57941f85de074de51ca368e21fc952262f4c3036 Mon Sep 17 00:00:00 2001 From: Cryoris Date: Mon, 2 Mar 2020 10:53:33 +0100 Subject: [PATCH 3/9] fix lint: additional linebreak --- test/python/circuit/test_parameters.py | 1 - 1 file changed, 1 deletion(-) diff --git a/test/python/circuit/test_parameters.py b/test/python/circuit/test_parameters.py index c819a7896d6f..3e69fb546266 100644 --- a/test/python/circuit/test_parameters.py +++ b/test/python/circuit/test_parameters.py @@ -122,7 +122,6 @@ def test_sorted_by_insertion(self, target): else: raise ValueError('Unsupported target {}'.format(target)) - def test_multiple_parameters(self): """Test setting multiple parameters""" theta = Parameter('θ') From 60458c74b617c7022ef72ffa6c71c85b73f24d26 Mon Sep 17 00:00:00 2001 From: Cryoris Date: Mon, 2 Mar 2020 11:39:52 +0100 Subject: [PATCH 4/9] cover the case python version < 3.6 --- qiskit/circuit/parametertable.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/qiskit/circuit/parametertable.py b/qiskit/circuit/parametertable.py index f2690dbf88ef..be4d452d3234 100644 --- a/qiskit/circuit/parametertable.py +++ b/qiskit/circuit/parametertable.py @@ -14,6 +14,8 @@ """ Look-up table for variable parameters in QuantumCircuit. """ +import sys +from collections import OrderedDict from collections.abc import MutableMapping from .instruction import Instruction @@ -27,7 +29,10 @@ def __init__(self, *args, **kwargs): the structure of _table is, {var_object: [(instruction_object, parameter_index), ...]} """ - self._table = dict(*args, **kwargs) + if sys.version_info.major >= 3 and sys.version_info.minor >= 6: + self._table = dict(*args, **kwargs) # since 3.6 dicts are insertion-ordered + else: + self._table = OrderedDict(*args, **kwargs) # otherwise use OrderedDict def __getitem__(self, key): return self._table[key] From d62837a068a9084d00eed2e4bc279c38b59ef49a Mon Sep 17 00:00:00 2001 From: Cryoris Date: Mon, 2 Mar 2020 14:41:05 +0100 Subject: [PATCH 5/9] fix sorted parameters for Python < 3.6 --- qiskit/circuit/quantumcircuit.py | 7 ++++--- qiskit/converters/circuit_to_gate.py | 23 ++++++++++++++------- qiskit/converters/circuit_to_instruction.py | 23 ++++++++++++++------- 3 files changed, 36 insertions(+), 17 deletions(-) diff --git a/qiskit/circuit/quantumcircuit.py b/qiskit/circuit/quantumcircuit.py index 48ce7bdcf4be..43c2ec5156d6 100644 --- a/qiskit/circuit/quantumcircuit.py +++ b/qiskit/circuit/quantumcircuit.py @@ -1238,8 +1238,6 @@ def from_qasm_str(qasm_str): @property def parameters(self): """Convenience function to get the parameters defined in the parameter table.""" - if len(set(self._parameter_table.keys())) != len(list(self._parameter_table.keys())): - raise ValueError('Some parameters existed twice!') return list(self._parameter_table.keys()) def bind_parameters(self, value_dict): @@ -1269,7 +1267,10 @@ def bind_parameters(self, value_dict): return new_circuit def _unroll_param_dict(self, value_dict): - unrolled_value_dict = {} + if sys.version_info.major >= 3 and sys.version_info.minor >= 6: + unrolled_value_dict = {} + else: + unrolled_value_dict = OrderedDict() for (param, value) in value_dict.items(): if isinstance(param, ParameterExpression): unrolled_value_dict[param] = value diff --git a/qiskit/converters/circuit_to_gate.py b/qiskit/converters/circuit_to_gate.py index 264dc011ea76..6799e62eb5b0 100644 --- a/qiskit/converters/circuit_to_gate.py +++ b/qiskit/converters/circuit_to_gate.py @@ -14,6 +14,9 @@ """Helper function for converting a circuit to a gate""" +import sys +from collections import OrderedDict + from qiskit.circuit.gate import Gate from qiskit.circuit.quantumregister import QuantumRegister, Qubit from qiskit.exceptions import QiskitError @@ -51,15 +54,21 @@ def circuit_to_gate(circuit, parameter_map=None): raise QiskitError('One or more instructions in this instruction ' 'cannot be converted to a gate') - if parameter_map is None: - parameter_dict = {p: p for p in circuit.parameters} + if sys.version_info.major >= 3 and sys.version_info.minor >= 6: + parameter_dict = {} else: - parameter_dict = circuit._unroll_param_dict(parameter_map) + parameter_dict = OrderedDict() - if parameter_dict.keys() != set(circuit.parameters): - raise QiskitError(('parameter_map should map all circuit parameters. ' - 'Circuit parameters: {}, parameter_map: {}').format( - circuit.parameters, parameter_dict)) + if parameter_map is None: + parameter_dict.update(zip(circuit.parameters, circuit.parameters)) + else: + unrolled_parameter_map = circuit._unroll_param_dict(parameter_map) + if unrolled_parameter_map.keys() != set(circuit.parameters): + raise QiskitError(('parameter_map should map all circuit parameters. ' + 'Circuit parameters: {}, parameter_map: {}').format( + circuit.parameters, parameter_dict)) + for parameter in circuit.parameters: + parameter_dict[parameter] = unrolled_parameter_map[parameter] gate = Gate(name=circuit.name, num_qubits=sum([qreg.size for qreg in circuit.qregs]), diff --git a/qiskit/converters/circuit_to_instruction.py b/qiskit/converters/circuit_to_instruction.py index b50895231ff7..6f99b6687061 100644 --- a/qiskit/converters/circuit_to_instruction.py +++ b/qiskit/converters/circuit_to_instruction.py @@ -14,6 +14,9 @@ """Helper function for converting a circuit to an instruction.""" +import sys +from collections import OrderedDict + from qiskit.exceptions import QiskitError from qiskit.circuit.instruction import Instruction from qiskit.circuit.quantumregister import QuantumRegister, Qubit @@ -59,15 +62,21 @@ def circuit_to_instruction(circuit, parameter_map=None): circuit_to_instruction(circ) """ - if parameter_map is None: - parameter_dict = {p: p for p in circuit.parameters} + if sys.version_info.major >= 3 and sys.version_info.minor >= 6: + parameter_dict = {} else: - parameter_dict = circuit._unroll_param_dict(parameter_map) + parameter_dict = OrderedDict() - if parameter_dict.keys() != set(circuit.parameters): - raise QiskitError(('parameter_map should map all circuit parameters. ' - 'Circuit parameters: {}, parameter_map: {}').format( - circuit.parameters, parameter_dict)) + if parameter_map is None: + parameter_dict.update(zip(circuit.parameters, circuit.parameters)) + else: + unrolled_parameter_map = circuit._unroll_param_dict(parameter_map) + if unrolled_parameter_map.keys() != set(circuit.parameters): + raise QiskitError(('parameter_map should map all circuit parameters. ' + 'Circuit parameters: {}, parameter_map: {}').format( + circuit.parameters, parameter_dict)) + for parameter in circuit.parameters: + parameter_dict[parameter] = unrolled_parameter_map[parameter] instruction = Instruction(name=circuit.name, num_qubits=sum([qreg.size for qreg in circuit.qregs]), From 083df32c332533655bd175902df712a59ec2bf96 Mon Sep 17 00:00:00 2001 From: Cryoris Date: Mon, 2 Mar 2020 14:42:59 +0100 Subject: [PATCH 6/9] fix lint --- qiskit/converters/circuit_to_gate.py | 2 +- qiskit/converters/circuit_to_instruction.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/qiskit/converters/circuit_to_gate.py b/qiskit/converters/circuit_to_gate.py index 6799e62eb5b0..3ff65bceac08 100644 --- a/qiskit/converters/circuit_to_gate.py +++ b/qiskit/converters/circuit_to_gate.py @@ -66,7 +66,7 @@ def circuit_to_gate(circuit, parameter_map=None): if unrolled_parameter_map.keys() != set(circuit.parameters): raise QiskitError(('parameter_map should map all circuit parameters. ' 'Circuit parameters: {}, parameter_map: {}').format( - circuit.parameters, parameter_dict)) + circuit.parameters, parameter_dict)) for parameter in circuit.parameters: parameter_dict[parameter] = unrolled_parameter_map[parameter] diff --git a/qiskit/converters/circuit_to_instruction.py b/qiskit/converters/circuit_to_instruction.py index 6f99b6687061..44bb450d769a 100644 --- a/qiskit/converters/circuit_to_instruction.py +++ b/qiskit/converters/circuit_to_instruction.py @@ -74,7 +74,7 @@ def circuit_to_instruction(circuit, parameter_map=None): if unrolled_parameter_map.keys() != set(circuit.parameters): raise QiskitError(('parameter_map should map all circuit parameters. ' 'Circuit parameters: {}, parameter_map: {}').format( - circuit.parameters, parameter_dict)) + circuit.parameters, parameter_dict)) for parameter in circuit.parameters: parameter_dict[parameter] = unrolled_parameter_map[parameter] From ccb4e6c4ceedd69d52cabe69e10002b626b51ed5 Mon Sep 17 00:00:00 2001 From: Cryoris Date: Tue, 3 Mar 2020 10:44:58 +0100 Subject: [PATCH 7/9] use OrderedDict throughout for readability can be changed to dict() when Python 3.5 reaches EOL for Qiskit --- qiskit/circuit/parametertable.py | 6 +----- qiskit/circuit/quantumcircuit.py | 5 +---- qiskit/converters/circuit_to_gate.py | 7 +------ qiskit/converters/circuit_to_instruction.py | 7 +------ 4 files changed, 4 insertions(+), 21 deletions(-) diff --git a/qiskit/circuit/parametertable.py b/qiskit/circuit/parametertable.py index be4d452d3234..72d00a523381 100644 --- a/qiskit/circuit/parametertable.py +++ b/qiskit/circuit/parametertable.py @@ -14,7 +14,6 @@ """ Look-up table for variable parameters in QuantumCircuit. """ -import sys from collections import OrderedDict from collections.abc import MutableMapping @@ -29,10 +28,7 @@ def __init__(self, *args, **kwargs): the structure of _table is, {var_object: [(instruction_object, parameter_index), ...]} """ - if sys.version_info.major >= 3 and sys.version_info.minor >= 6: - self._table = dict(*args, **kwargs) # since 3.6 dicts are insertion-ordered - else: - self._table = OrderedDict(*args, **kwargs) # otherwise use OrderedDict + self._table = OrderedDict(*args, **kwargs) # replace by dict() when Python 3.5 reaches EOL def __getitem__(self, key): return self._table[key] diff --git a/qiskit/circuit/quantumcircuit.py b/qiskit/circuit/quantumcircuit.py index 43c2ec5156d6..1ab795a5a4b1 100644 --- a/qiskit/circuit/quantumcircuit.py +++ b/qiskit/circuit/quantumcircuit.py @@ -1267,10 +1267,7 @@ def bind_parameters(self, value_dict): return new_circuit def _unroll_param_dict(self, value_dict): - if sys.version_info.major >= 3 and sys.version_info.minor >= 6: - unrolled_value_dict = {} - else: - unrolled_value_dict = OrderedDict() + unrolled_value_dict = OrderedDict() # replace by dict() when Python 3.5 reaches EOL for (param, value) in value_dict.items(): if isinstance(param, ParameterExpression): unrolled_value_dict[param] = value diff --git a/qiskit/converters/circuit_to_gate.py b/qiskit/converters/circuit_to_gate.py index 3ff65bceac08..dfc3d0dd4858 100644 --- a/qiskit/converters/circuit_to_gate.py +++ b/qiskit/converters/circuit_to_gate.py @@ -14,7 +14,6 @@ """Helper function for converting a circuit to a gate""" -import sys from collections import OrderedDict from qiskit.circuit.gate import Gate @@ -54,11 +53,7 @@ def circuit_to_gate(circuit, parameter_map=None): raise QiskitError('One or more instructions in this instruction ' 'cannot be converted to a gate') - if sys.version_info.major >= 3 and sys.version_info.minor >= 6: - parameter_dict = {} - else: - parameter_dict = OrderedDict() - + parameter_dict = OrderedDict() # replace by dict() when Python 3.5 reaches EOL if parameter_map is None: parameter_dict.update(zip(circuit.parameters, circuit.parameters)) else: diff --git a/qiskit/converters/circuit_to_instruction.py b/qiskit/converters/circuit_to_instruction.py index 44bb450d769a..8536d1a83350 100644 --- a/qiskit/converters/circuit_to_instruction.py +++ b/qiskit/converters/circuit_to_instruction.py @@ -14,7 +14,6 @@ """Helper function for converting a circuit to an instruction.""" -import sys from collections import OrderedDict from qiskit.exceptions import QiskitError @@ -62,11 +61,7 @@ def circuit_to_instruction(circuit, parameter_map=None): circuit_to_instruction(circ) """ - if sys.version_info.major >= 3 and sys.version_info.minor >= 6: - parameter_dict = {} - else: - parameter_dict = OrderedDict() - + parameter_dict = OrderedDict() # replace by dict() when Python 3.5 reaches EOL if parameter_map is None: parameter_dict.update(zip(circuit.parameters, circuit.parameters)) else: From 58fe9283300c13582f7262a9a25f01626ac5be6c Mon Sep 17 00:00:00 2001 From: Cryoris Date: Wed, 4 Mar 2020 10:17:39 +0100 Subject: [PATCH 8/9] sorting by name as default, insertion on demand --- qiskit/circuit/quantumcircuit.py | 14 +++++-- qiskit/converters/circuit_to_gate.py | 12 +++++- qiskit/converters/circuit_to_instruction.py | 12 +++++- test/python/circuit/test_parameters.py | 42 ++++++++++++++----- .../converters/test_circuit_to_instruction.py | 2 +- 5 files changed, 63 insertions(+), 19 deletions(-) diff --git a/qiskit/circuit/quantumcircuit.py b/qiskit/circuit/quantumcircuit.py index 1ab795a5a4b1..93538a6f0a82 100644 --- a/qiskit/circuit/quantumcircuit.py +++ b/qiskit/circuit/quantumcircuit.py @@ -568,7 +568,7 @@ def _check_cargs(self, cargs): if not all(self.has_register(i.register) for i in cargs): raise CircuitError("register not in this circuit") - def to_instruction(self, parameter_map=None): + def to_instruction(self, parameter_map=None, sort_parameters_by_name=True): """Create an Instruction out of this circuit. Args: @@ -576,15 +576,18 @@ def to_instruction(self, parameter_map=None): parameters in the circuit to parameters to be used in the instruction. If None, existing circuit parameters will also parameterize the instruction. + sort_parameters_by_name (bool): If True, the parameters in the circuit are sorted by + name before being added to the gate. Otherwise, the order of the circuit is used, + i.e. insertion-ordered by default. Returns: Instruction: a composite instruction encapsulating this circuit (can be decomposed back) """ from qiskit.converters.circuit_to_instruction import circuit_to_instruction - return circuit_to_instruction(self, parameter_map) + return circuit_to_instruction(self, parameter_map, sort_parameters_by_name) - def to_gate(self, parameter_map=None): + def to_gate(self, parameter_map=None, sort_parameters_by_name=True): """Create a Gate out of this circuit. Args: @@ -592,13 +595,16 @@ def to_gate(self, parameter_map=None): parameters in the circuit to parameters to be used in the gate. If None, existing circuit parameters will also parameterize the gate. + sort_parameters_by_name (bool): If True, the parameters in the circuit are sorted by + name before being added to the gate. Otherwise, the order of the circuit is used, + i.e. insertion-ordered by default. Returns: Gate: a composite gate encapsulating this circuit (can be decomposed back) """ from qiskit.converters.circuit_to_gate import circuit_to_gate - return circuit_to_gate(self, parameter_map) + return circuit_to_gate(self, parameter_map, sort_parameters_by_name) def decompose(self): """Call a decomposition pass on this circuit, diff --git a/qiskit/converters/circuit_to_gate.py b/qiskit/converters/circuit_to_gate.py index dfc3d0dd4858..84edb9694ae9 100644 --- a/qiskit/converters/circuit_to_gate.py +++ b/qiskit/converters/circuit_to_gate.py @@ -21,7 +21,7 @@ from qiskit.exceptions import QiskitError -def circuit_to_gate(circuit, parameter_map=None): +def circuit_to_gate(circuit, parameter_map=None, sort_parameters_by_name=True): """Build a ``Gate`` object from a ``QuantumCircuit``. The gate is anonymous (not tied to a named quantum register), @@ -34,6 +34,9 @@ def circuit_to_gate(circuit, parameter_map=None): parameters in the circuit to parameters to be used in the gate. If None, existing circuit parameters will also parameterize the Gate. + sort_parameters_by_name (bool): If True, the parameters in the circuit are sorted by name + before being added to the gate. Otherwise, the order of the circuit is used, i.e. + insertion-ordered by default. Raises: QiskitError: if circuit is non-unitary or if @@ -65,9 +68,14 @@ def circuit_to_gate(circuit, parameter_map=None): for parameter in circuit.parameters: parameter_dict[parameter] = unrolled_parameter_map[parameter] + if sort_parameters_by_name: + gate_parameters = sorted(parameter_dict.values(), key=lambda p: p.name) + else: + gate_parameters = list(parameter_dict.values()) + gate = Gate(name=circuit.name, num_qubits=sum([qreg.size for qreg in circuit.qregs]), - params=list(parameter_dict.values())) + params=gate_parameters) gate.condition = None def find_bit_position(bit): diff --git a/qiskit/converters/circuit_to_instruction.py b/qiskit/converters/circuit_to_instruction.py index 8536d1a83350..787e8ecb1f0d 100644 --- a/qiskit/converters/circuit_to_instruction.py +++ b/qiskit/converters/circuit_to_instruction.py @@ -22,7 +22,7 @@ from qiskit.circuit.classicalregister import ClassicalRegister -def circuit_to_instruction(circuit, parameter_map=None): +def circuit_to_instruction(circuit, parameter_map=None, sort_parameters_by_name=True): """Build an ``Instruction`` object from a ``QuantumCircuit``. The instruction is anonymous (not tied to a named quantum register), @@ -35,6 +35,9 @@ def circuit_to_instruction(circuit, parameter_map=None): parameters in the circuit to parameters to be used in the instruction. If None, existing circuit parameters will also parameterize the instruction. + sort_parameters_by_name (bool): If True, the parameters in the circuit are sorted by name + before being added to the gate. Otherwise, the order of the circuit is used, i.e. + insertion-ordered by default. Raises: QiskitError: if parameter_map is not compatible with circuit @@ -73,10 +76,15 @@ def circuit_to_instruction(circuit, parameter_map=None): for parameter in circuit.parameters: parameter_dict[parameter] = unrolled_parameter_map[parameter] + if sort_parameters_by_name: + gate_parameters = sorted(parameter_dict.values(), key=lambda p: p.name) + else: + gate_parameters = list(parameter_dict.values()) + instruction = Instruction(name=circuit.name, num_qubits=sum([qreg.size for qreg in circuit.qregs]), num_clbits=sum([creg.size for creg in circuit.cregs]), - params=list(parameter_dict.values())) + params=gate_parameters) instruction.condition = None def find_bit_position(bit): diff --git a/test/python/circuit/test_parameters.py b/test/python/circuit/test_parameters.py index 3e69fb546266..909f09094306 100644 --- a/test/python/circuit/test_parameters.py +++ b/test/python/circuit/test_parameters.py @@ -107,11 +107,11 @@ def test_sorted_by_insertion(self, target): if target == 'circuit': self.assertEqual(qc.parameters, params) elif target == 'gate': - self.assertEqual(qc.to_gate().params, params) + self.assertEqual(qc.to_gate(sort_parameters_by_name=False).params, params) elif target == 'instruction': - self.assertEqual(qc.to_gate().params, params) + self.assertEqual(qc.to_gate(sort_parameters_by_name=False).params, params) elif target == 'nested': - gate = qc.to_gate() + gate = qc.to_gate(sort_parameters_by_name=False) qc2 = QuantumCircuit(2) before, after = Parameter('before'), Parameter('after') qc2.rz(before, 1) @@ -130,7 +130,7 @@ def test_multiple_parameters(self): qc = QuantumCircuit(qr) qc.rx(theta, qr) qc.u3(0, theta, x, qr) - self.assertEqual(qc.parameters, [theta, x]) + self.assertEqual(set(qc.parameters), {theta, x}) def test_partial_binding(self): """Test that binding a subset of circuit parameters returns a new parameterized circuit.""" @@ -254,7 +254,7 @@ def test_circuit_composition(self): qc2.measure(qr, cr) qc3 = qc1 + qc2 - self.assertEqual(qc3.parameters, [theta, phi]) + self.assertEqual(set(qc3.parameters), {theta, phi}) def test_composite_instruction(self): """Test preservation of parameters via parameterized instructions.""" @@ -273,7 +273,7 @@ def test_composite_instruction(self): qc2.ry(phi, qr2[0]) qc2.h(qr2) qc2.append(gate, qargs=[qr2[1]]) - self.assertEqual(qc2.parameters, [phi, theta]) + self.assertEqual(set(qc2.parameters), {phi, theta}) def test_parameter_name_conflicts_raises(self): """Verify attempting to add different parameters with matching names raises an error.""" @@ -778,7 +778,8 @@ def test_to_instruction_with_expression(self, target_type, order): elif target_type == 'instruction': gate = qc1.to_instruction() - self.assertEqual(gate.params, [theta, phi]) + # parameters are name sorted by default, thus [phi, θ] + self.assertEqual(gate.params, [phi, theta]) delta = Parameter('delta') qr2 = QuantumRegister(3, name='qr2') @@ -786,7 +787,17 @@ def test_to_instruction_with_expression(self, target_type, order): qc2.ry(delta, qr2[0]) qc2.append(gate, qargs=[qr2[1]]) - self.assertEqual(qc2.parameters, [delta, theta, phi]) + with self.subTest(msg='test contained parameters'): + self.assertEqual(set(qc2.parameters), {delta, theta, phi}) + + with self.subTest(msg='test order of the parameters'): + # Within the circuit the parameters are insertion ordered. + # Since first ry(delta) is applied, this is the first parameter. Then `gate` is + # appended which has the parameters [phi, theta]. Note that the conversion to + # a gate assumes sorting of the parameters by name, which can be turned off + # using `QuantumCircuit.to_gate(sort_parameters_by_name=False)`. Then the insertion + # ordering of the circuit is also used in the gate. + self.assertEqual(qc2.parameters, [delta, phi, theta]) binds = {delta: 1, theta: 2, phi: 3} expected_qc = QuantumCircuit(qr2) @@ -825,7 +836,8 @@ def test_to_instruction_expression_parameter_map(self, target_type, order): elif target_type == 'instruction': gate = qc1.to_instruction(parameter_map={theta: theta_p, phi: phi_p}) - self.assertEqual(gate.params, [theta_p, phi_p]) + # parameters are name sorted by default, thus [phi, theta] + self.assertEqual(gate.params, [phi_p, theta_p]) delta = Parameter('delta') qr2 = QuantumRegister(3, name='qr2') @@ -833,7 +845,17 @@ def test_to_instruction_expression_parameter_map(self, target_type, order): qc2.ry(delta, qr2[0]) qc2.append(gate, qargs=[qr2[1]]) - self.assertEqual(qc2.parameters, [delta, theta_p, phi_p]) + with self.subTest(msg='test contained parameters'): + self.assertEqual(set(qc2.parameters), {delta, theta_p, phi_p}) + + with self.subTest(msg='test order of the parameters'): + # Within the circuit the parameters are insertion ordered. + # Since first ry(delta) is applied, this is the first parameter. Then `gate` is + # appended which has the parameters [phi, theta]. Note that the conversion to + # a gate assumes sorting of the parameters by name, which can be turned off + # using `QuantumCircuit.to_gate(sort_parameters_by_name=False)`. Then the insertion + # ordering of the circuit is also used in the gate. + self.assertEqual(qc2.parameters, [delta, phi_p, theta_p]) binds = {delta: 1, theta_p: 2, phi_p: 3} expected_qc = QuantumCircuit(qr2) diff --git a/test/python/converters/test_circuit_to_instruction.py b/test/python/converters/test_circuit_to_instruction.py index 339f80894d08..1342eab044c4 100644 --- a/test/python/converters/test_circuit_to_instruction.py +++ b/test/python/converters/test_circuit_to_instruction.py @@ -61,7 +61,7 @@ def test_flatten_parameters(self): inst = circuit_to_instruction(qc) - self.assertEqual(inst.params, [theta, phi]) + self.assertEqual(inst.params, [phi, theta]) self.assertEqual(inst.definition[0][0].params, [theta]) self.assertEqual(inst.definition[1][0].params, [phi]) self.assertEqual(inst.definition[2][0].params, [theta, phi]) From 60f7bcf2190a5adf8c732ed396fa42b95066c315 Mon Sep 17 00:00:00 2001 From: Cryoris Date: Wed, 4 Mar 2020 12:05:50 +0100 Subject: [PATCH 9/9] fix test bugs introduced from merge master --- test/python/circuit/test_parameters.py | 29 +++++++++++++------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/test/python/circuit/test_parameters.py b/test/python/circuit/test_parameters.py index 96b1065ce87e..c74c59c91baf 100644 --- a/test/python/circuit/test_parameters.py +++ b/test/python/circuit/test_parameters.py @@ -481,18 +481,22 @@ def test_decompose_propagates_bound_parameters(self, target_type, parameter_type qc2 = QuantumCircuit(1) qc2.append(inst, [0]) - bound_qc2 = qc2.bind_parameters({theta: 0.5}) - - self.assertEqual(qc2.parameters, [theta]) - self.assertEqual(bound_qc2.parameters, []) + expected_qc2 = QuantumCircuit(1) + if parameter_type == 'numbers': + bound_qc2 = qc2.bind_parameters({theta: 0.5}) + expected_parameters = [] + expected_qc2.rx(0.5, 0) + else: + phi = Parameter('ph') + bound_qc2 = qc2.copy() + bound_qc2._substitute_parameters({theta: phi}) + expected_parameters = [phi] + expected_qc2.rx(phi, 0) decomposed_qc2 = bound_qc2.decompose() with self.subTest(msg='testing parameters of initial circuit'): - self.assertEqual(qc2.parameters, {theta}) - - self.assertEqual(decomposed_qc2.parameters, []) - self.assertEqual(decomposed_qc2, expected_qc2) + self.assertEqual(qc2.parameters, [theta]) with self.subTest(msg='testing parameters of deep decomposed bound circuit'): self.assertEqual(decomposed_qc2.parameters, expected_parameters) @@ -525,21 +529,21 @@ def test_decompose_propagates_deeply_bound_parameters(self, target_type, paramet if parameter_type == 'numbers': bound_qc3 = qc3.bind_parameters({theta: 0.5}) - expected_parameters = set() + expected_parameters = [] expected_qc3 = QuantumCircuit(1) expected_qc3.rx(0.5, 0) else: phi = Parameter('ph') bound_qc3 = qc3.copy() bound_qc3._substitute_parameters({theta: phi}) - expected_parameters = {phi} + expected_parameters = [phi] expected_qc3 = QuantumCircuit(1) expected_qc3.rx(phi, 0) deep_decomposed_qc3 = bound_qc3.decompose().decompose() self.assertEqual(qc3.parameters, [theta]) - self.assertEqual(bound_qc3.parameters, []) + self.assertEqual(bound_qc3.parameters, expected_parameters) with self.subTest(msg='testing parameters of bound circuit'): self.assertEqual(bound_qc3.parameters, expected_parameters) @@ -547,9 +551,6 @@ def test_decompose_propagates_deeply_bound_parameters(self, target_type, paramet with self.subTest(msg='testing parameters of deep decomposed bound circuit'): self.assertEqual(deep_decomposed_qc3.parameters, expected_parameters) - self.assertEqual(deep_decomposed_qc3.parameters, []) - self.assertEqual(deep_decomposed_qc3, expected_qc3) - @data('gate', 'instruction') def test_executing_parameterized_instruction_bound_early(self, target_type): """Verify bind-before-execute preserves bound values."""