Skip to content

Commit 41a35f5

Browse files
committed
Remove QPY warning on deserialising incomplete ParameterVectors
This warning was added with QPY v3[^1] at a time when `ParameterVector` generated all its elements with unrelated UUIDs. Since `qiskit-terra` v0.25, we actually _have_ had enough information to completely recreate an entire vector from a single element[^2], and the circumstances needed to observe the incomplete behaviour are particularly convoluted, and unlikely in practice. [^1]: 5c4cd2b: Fix ParameterVector handling in QPY serialization (gh-7381) [^2]: 5ab231d: Slightly optimize ParameterVector construction (gh-10403)
1 parent 3767817 commit 41a35f5

7 files changed

Lines changed: 45 additions & 151 deletions

File tree

crates/qpy/src/circuit_reader.rs

Lines changed: 0 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1518,35 +1518,6 @@ pub(crate) fn unpack_circuit(
15181518
let inst = unpack_instruction(instruction, &custom_instructions, &mut qpy_data)?;
15191519
qpy_data.circuit_data.push(inst)?;
15201520
}
1521-
for (vector, initialized_params) in qpy_data.vectors.values() {
1522-
let vector_length = vector
1523-
.bind(py)
1524-
.call_method0("__len__")?
1525-
.extract::<usize>()?;
1526-
let missing_indices: Vec<u64> = (0..vector_length as u64)
1527-
.filter(|x| !initialized_params.contains(&(*x as u32)))
1528-
.collect();
1529-
if initialized_params.len() != vector_length {
1530-
let msg = format!(
1531-
"The ParameterVector: '{:}' is not fully identical to its \
1532-
pre-serialization state. Elements {:} \
1533-
in the ParameterVector will be not equal to the pre-serialized ParameterVector \
1534-
as they weren't used in the circuit: {:}",
1535-
vector.getattr(py, "name")?.extract::<String>(py)?,
1536-
missing_indices
1537-
.iter()
1538-
.map(|index| index.to_string())
1539-
.collect::<Vec<_>>()
1540-
.join(", "),
1541-
packed_circuit.header.circuit_name
1542-
);
1543-
imports::WARNINGS_WARN.get_bound(py).call1((
1544-
msg,
1545-
imports::BUILTIN_USER_WARNING.get_bound(py),
1546-
1,
1547-
))?;
1548-
}
1549-
}
15501521
// since we don't have a rust QuantumCircuit, and the metadata and custom layouts are also in python
15511522
// this pythonic part is unavoidable
15521523
let unpacked_layout = unpack_layout(py, &packed_circuit.layout, &circuit_data)?;

crates/qpy/src/params.rs

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -543,7 +543,7 @@ pub(crate) fn unpack_parameter_vector(
543543

544544
let vector = Python::attach(|py| -> Result<_, QpyError> {
545545
// we use python-space to interface with the ParameterVector data
546-
let vector_data = match qpy_data.vectors.get_mut(&root_uuid) {
546+
let vector = match qpy_data.vectors.get(&root_uuid) {
547547
Some(value) => value,
548548
None => Python::attach(|py| -> Result<_, QpyError> {
549549
// we use python-space to create a new parameter vector
@@ -557,17 +557,13 @@ pub(crate) fn unpack_parameter_vector(
557557
.get_bound(py)
558558
.call1((name.clone(), parameter_vector_pack.vector_size, py_uuid))?
559559
.unbind();
560-
qpy_data
561-
.vectors
562-
.insert(root_uuid, (vector, Default::default()));
563-
qpy_data.vectors.get_mut(&root_uuid).ok_or_else(|| {
560+
qpy_data.vectors.insert(root_uuid, vector);
561+
qpy_data.vectors.get(&root_uuid).ok_or_else(|| {
564562
QpyError::MissingData("Parameter vector creation failed".to_string())
565563
})
566564
})?,
567565
};
568-
let vector = vector_data.0.bind(py);
569-
vector_data.1.insert(index);
570-
Ok(vector.clone().unbind())
566+
Ok(vector.clone_ref(py))
571567
})?;
572568

573569
Ok(Symbol {

crates/qpy/src/value.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ use std::sync::Arc;
1515

1616
use binrw::meta::{ReadEndian, WriteEndian};
1717
use binrw::{BinRead, BinWrite, Endian, binrw};
18-
use hashbrown::{HashMap, HashSet};
18+
use hashbrown::HashMap;
1919
use pyo3::prelude::*;
2020
use pyo3::types::PyAny;
2121

@@ -126,7 +126,7 @@ pub struct QPYReadData<'a> {
126126
pub use_symengine: bool,
127127
pub standalone_vars: HashMap<u16, qiskit_circuit::Var>,
128128
pub standalone_stretches: HashMap<u16, qiskit_circuit::Stretch>,
129-
pub vectors: HashMap<Uuid, (Py<PyAny>, HashSet<u32>)>, // Parameter expression vectors, which are a python-only elements for now
129+
pub vectors: HashMap<Uuid, Py<PyAny>>, // Parameter expression vectors, which are a python-only elements for now
130130
pub annotation_handler: AnnotationHandler<'a>,
131131
}
132132

qiskit/qpy/binary_io/circuits.py

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1805,16 +1805,6 @@ def read_circuit(
18051805
if version >= 5:
18061806
_read_calibrations(file_obj, version, vectors, metadata_deserializer)
18071807

1808-
for vector, initialized_params in vectors.values():
1809-
if len(initialized_params) != len(vector):
1810-
warnings.warn(
1811-
f"The ParameterVector: '{vector.name}' is not fully identical to its "
1812-
"pre-serialization state. Elements "
1813-
f"{', '.join([str(x) for x in set(range(len(vector))) - initialized_params])} "
1814-
"in the ParameterVector will be not equal to the pre-serialized ParameterVector "
1815-
f"as they weren't used in the circuit: {circ.name}",
1816-
UserWarning,
1817-
)
18181808
if version >= 8:
18191809
if version >= 10:
18201810
_read_layout_v2(file_obj, circ)

qiskit/qpy/binary_io/value.py

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -458,11 +458,8 @@ def _read_parameter_vec(file_obj, vectors):
458458
root_uuid_int = uuid.UUID(bytes=data.uuid).int - data.index
459459
root_uuid = uuid.UUID(int=root_uuid_int)
460460
name = file_obj.read(data.vector_name_size).decode(common.ENCODE)
461-
462-
if root_uuid not in vectors:
463-
vectors[root_uuid] = (ParameterVector(name, data.vector_size, uuid=root_uuid), set())
464-
vector = vectors[root_uuid][0]
465-
vectors[root_uuid][1].add(data.index)
461+
if (vector := vectors.get(root_uuid, None)) is None:
462+
vector = vectors[root_uuid] = ParameterVector(name, data.vector_size, uuid=root_uuid)
466463
return vector[data.index]
467464

468465

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
---
2+
fixes:
3+
- QPY deserialization no longer emits a warning when loading a circuit that uses some, but not all,
4+
elements of a :class:`.ParameterVector`.
5+
6+
issues:
7+
- |
8+
QPY files generated by ``qiskit-terra`` v0.24 and prior may define :class:`.ParameterVector`
9+
instances whose elements have disconnected :attr:`.Parameter.uuid`
10+
attributes, which cannot be completely round-tripped. Prior to this version of Qiskit, QPY
11+
would emit a warning if it detected an implicit :class:`.ParameterVector` that had not been
12+
completely reconstructed. QPY will no longer warn in this situtation.
13+
14+
In order to observe the dangerous behavior, all the following are required:
15+
16+
* you load two separate QPY circuit payloads (in the same file, or separate files) generated by
17+
``qiskit-terra`` v0.24 or earlier.
18+
* the two circuits use :class:`.ParameterVectorElement` instances derived from the same
19+
:class:`.ParameterVector`.
20+
* one circuit uses a particular element from the vector, and the other does not use it.
21+
* you attempt to compare the directly deserialized :class:`.ParameterVectorElement` from the
22+
circuit that used it to the same element index, but retrieved through the
23+
:attr:`.ParameterVectorElement.vector` backreference of a different element from the circuit
24+
that does not use the compared element.
25+
26+
The warning has been removed because in modern Qiskit it is a false positive far more often than
27+
it indicates an observed problem. Qiskit v2.5 onwards in fact will create an implicit
28+
:class:`.ParameterVector` whose elements compare equal between circuits.

test/python/circuit/test_circuit_load_from_qpy.py

Lines changed: 9 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,6 @@
1919
import random
2020
import tempfile
2121
import unittest
22-
import warnings
23-
import re
2422

2523
import ddt
2624
import numpy as np
@@ -925,16 +923,7 @@ def test_pauli_feature_map_legacy(self):
925923
qpy_file = io.BytesIO()
926924
dump(qc, qpy_file)
927925
qpy_file.seek(0)
928-
with warnings.catch_warnings(record=True) as w:
929-
warnings.simplefilter("always")
930-
new_circuit = load(qpy_file)[0]
931-
for warning in w:
932-
self.assertFalse(
933-
re.search(
934-
r"is not fully identical to its pre-serialization state",
935-
str(warning.message),
936-
)
937-
)
926+
new_circuit = load(qpy_file)[0]
938927
self.assertEqual(qc, new_circuit)
939928

940929
def test_pauli_feature_map_new(self):
@@ -945,16 +934,7 @@ def test_pauli_feature_map_new(self):
945934
qpy_file = io.BytesIO()
946935
dump(qc, qpy_file)
947936
qpy_file.seek(0)
948-
with warnings.catch_warnings(record=True) as w:
949-
warnings.simplefilter("always")
950-
new_circuit = load(qpy_file)[0]
951-
for warning in w:
952-
self.assertFalse(
953-
re.search(
954-
r"is not fully identical to its pre-serialization state",
955-
str(warning.message),
956-
)
957-
)
937+
new_circuit = load(qpy_file)[0]
958938
self.assertEqual(qc, new_circuit)
959939

960940
def test_zz_feature_map_legacy(self):
@@ -966,16 +946,7 @@ def test_zz_feature_map_legacy(self):
966946
qpy_file = io.BytesIO()
967947
dump(qc, qpy_file)
968948
qpy_file.seek(0)
969-
with warnings.catch_warnings(record=True) as w:
970-
warnings.simplefilter("always")
971-
new_circuit = load(qpy_file)[0]
972-
for warning in w:
973-
self.assertFalse(
974-
re.search(
975-
r"is not fully identical to its pre-serialization state",
976-
str(warning.message),
977-
)
978-
)
949+
new_circuit = load(qpy_file)[0]
979950
self.assertEqual(qc, new_circuit)
980951

981952
def test_zz_feature_map_new(self):
@@ -986,16 +957,7 @@ def test_zz_feature_map_new(self):
986957
qpy_file = io.BytesIO()
987958
dump(qc, qpy_file)
988959
qpy_file.seek(0)
989-
with warnings.catch_warnings(record=True) as w:
990-
warnings.simplefilter("always")
991-
new_circuit = load(qpy_file)[0]
992-
for warning in w:
993-
self.assertFalse(
994-
re.search(
995-
r"is not fully identical to its pre-serialization state",
996-
str(warning.message),
997-
)
998-
)
960+
new_circuit = load(qpy_file)[0]
999961
self.assertEqual(qc, new_circuit)
1000962

1001963
def test_real_amplitudes_legacy(self):
@@ -1007,16 +969,7 @@ def test_real_amplitudes_legacy(self):
1007969
qpy_file = io.BytesIO()
1008970
dump(qc, qpy_file)
1009971
qpy_file.seek(0)
1010-
with warnings.catch_warnings(record=True) as w:
1011-
warnings.simplefilter("always")
1012-
new_circuit = load(qpy_file)[0]
1013-
for warning in w:
1014-
self.assertFalse(
1015-
re.search(
1016-
r"is not fully identical to its pre-serialization state",
1017-
str(warning.message),
1018-
)
1019-
)
972+
new_circuit = load(qpy_file)[0]
1020973
self.assertEqual(qc, new_circuit)
1021974

1022975
def test_real_amplitudes_new(self):
@@ -1027,16 +980,7 @@ def test_real_amplitudes_new(self):
1027980
qpy_file = io.BytesIO()
1028981
dump(qc, qpy_file)
1029982
qpy_file.seek(0)
1030-
with warnings.catch_warnings(record=True) as w:
1031-
warnings.simplefilter("always")
1032-
new_circuit = load(qpy_file)[0]
1033-
for warning in w:
1034-
self.assertFalse(
1035-
re.search(
1036-
r"is not fully identical to its pre-serialization state",
1037-
str(warning.message),
1038-
)
1039-
)
983+
new_circuit = load(qpy_file)[0]
1040984
self.assertEqual(qc, new_circuit)
1041985

1042986
def test_duplicated_param_name_legacy(self):
@@ -1053,16 +997,7 @@ def test_duplicated_param_name_legacy(self):
1053997
qpy_file = io.BytesIO()
1054998
dump(qc, qpy_file)
1055999
qpy_file.seek(0)
1056-
with warnings.catch_warnings(record=True) as w:
1057-
warnings.simplefilter("always")
1058-
new_circuit = load(qpy_file)[0]
1059-
for warning in w:
1060-
self.assertFalse(
1061-
re.search(
1062-
r"is not fully identical to its pre-serialization state",
1063-
str(warning.message),
1064-
)
1065-
)
1000+
new_circuit = load(qpy_file)[0]
10661001
self.assertEqual(qc, new_circuit)
10671002

10681003
def test_duplicated_param_name_new(self):
@@ -1078,16 +1013,7 @@ def test_duplicated_param_name_new(self):
10781013
qpy_file = io.BytesIO()
10791014
dump(qc, qpy_file)
10801015
qpy_file.seek(0)
1081-
with warnings.catch_warnings(record=True) as w:
1082-
warnings.simplefilter("always")
1083-
new_circuit = load(qpy_file)[0]
1084-
for warning in w:
1085-
self.assertFalse(
1086-
re.search(
1087-
r"is not fully identical to its pre-serialization state",
1088-
str(warning.message),
1089-
)
1090-
)
1016+
new_circuit = load(qpy_file)[0]
10911017
self.assertEqual(qc, new_circuit)
10921018

10931019
def test_parameter_expression_global_phase(self):
@@ -1165,8 +1091,7 @@ def dump_load_param_vec(qc):
11651091
x = ParameterVector("γ", 2)
11661092
qc = QuantumCircuit(3)
11671093
qc.rzz(x[0], 0, 1)
1168-
with self.assertWarns(UserWarning):
1169-
params, new_params, vector, new_vector = dump_load_param_vec(qc)
1094+
params, new_params, vector, new_vector = dump_load_param_vec(qc)
11701095

11711096
self.assertTrue(all(p == q for p, q in zip(params, new_params)))
11721097
# vector[0] is part of the circuit
@@ -1209,19 +1134,6 @@ def test_parameter_vector_element_in_expression(self):
12091134
self.assertEqual([x.name for x in new_circuit.parameters], expected_params)
12101135
self.assertDeprecatedBitProperties(qc, new_circuit)
12111136

1212-
def test_parameter_vector_incomplete_warns(self):
1213-
"""Test that qpy's deserialization warns if a ParameterVector isn't fully identical."""
1214-
vec = ParameterVector("test", 3)
1215-
qc = QuantumCircuit(1, name="fun")
1216-
qc.rx(vec[1], 0)
1217-
qpy_file = io.BytesIO()
1218-
dump(qc, qpy_file)
1219-
qpy_file.seek(0)
1220-
with self.assertWarnsRegex(UserWarning, r"^The ParameterVector.*Elements 0, 2.*fun$"):
1221-
new_circuit = load(qpy_file)[0]
1222-
self.assertEqual(qc, new_circuit)
1223-
self.assertDeprecatedBitProperties(qc, new_circuit)
1224-
12251137
def test_parameter_vector_global_phase(self):
12261138
"""Test that a circuit with a standalone ParameterVectorElement phase works."""
12271139
vec = ParameterVector("phase", 1)

0 commit comments

Comments
 (0)