Narrow QPY 18 SPARSE_OBSERVABLE bit terms from uint16_t to uint8_t

crates/qpy/src/circuit_reader.rs

@@ -1227,8 +1227,7 @@ fn deserialize_pauli_evolution_gate(
                     .bitterm_data
                     .iter()
                     .map(|&bitterm| -> Result<_, QpyError> {
-                        let reduced_bitterm = u8::try_from(bitterm)?;
-                        BitTerm::try_from(reduced_bitterm).map_err(|_| {
+                        BitTerm::try_from(bitterm).map_err(|_| {
                             QpyError::DeserializationError(
                                 "Could not read sparse observable data".to_string(),
                             )

crates/qpy/src/circuit_writer.rs

@@ -57,7 +57,7 @@ use crate::value::{
     BitType, CircuitInstructionType, ExpressionVarDeclaration, GenericValue, ParamRegisterValue,
     QPYWriteData, RegisterType, ValueEndian, get_circuit_type_key, pack_for_collection,
     pack_generic_value, pack_standalone_var, pack_stretch, serialize,
-    serialize_param_register_value,
+    serialize_param_register_value, serialize_with_args,
 };
 
 use qiskit_circuit::var_stretch_container::{StretchType, VarType};
@@ -1121,8 +1121,9 @@ fn pack_custom_instruction(
     let mut base_gate_raw: Bytes = Bytes::new();
 
     let data = match gate_type {
-        CircuitInstructionType::PauliEvolutionGate => Some(serialize(
+        CircuitInstructionType::PauliEvolutionGate => Some(serialize_with_args(
             &py_pack_pauli_evolution_gate(inst.ob.bind(py), qpy_data)?,
+            (qpy_data.version,),
         )?),
         CircuitInstructionType::ControlledGate => {
             // For ControlledGate, we have to access and store the private `_definition` rather than

crates/qpy/src/formats.rs

@@ -498,7 +498,7 @@ pub struct GenericDataSequencePack {
 #[binrw]
 #[brw(big)]
 #[derive(Debug)]
-#[br(import (version: u8))]
+#[brw(import (version: u8))]
 pub struct PauliEvolutionDefPack {
     #[bw(calc = pauli_data.len() as u64)]
     pub operator_size: u64,
@@ -509,6 +509,7 @@ pub struct PauliEvolutionDefPack {
     #[bw(calc = synth_data.len() as u64)]
     pub synth_method_size: u64,
     #[br(count = operator_size, args { inner: (version,) })]
+    #[bw(args(version))]
     pub pauli_data: Vec<PauliDataPack>,
     #[br(count = time_size)]
     pub time_data: Bytes,
@@ -519,14 +520,19 @@ pub struct PauliEvolutionDefPack {
 // A pauli operator data for pauli evolution gates
 // The operator is given either as a SparesePauliOp list or as a SparasePauliObservable
 // SparsePauliObservable was added in V17
+//
+// This variant covers V17 *and later*: the only difference from V18 onwards is the width of the
+// bit terms inside `SparsePauliObservableElemPack`, which that struct handles itself given
+// `version`.
 #[binrw]
 #[brw(big)]
 #[derive(Debug)]
+#[brw(import(version: u8))]
 pub enum PauliDataPackV17 {
     #[brw(magic = 0u8)] // old style: sparse pauli op list
     SparsePauliOp(SparsePauliOpListElemPack),
     #[brw(magic = 1u8)] // new style added in v17: sparse observable
-    SparseObservable(SparsePauliObservableElemPack),
+    SparseObservable(#[brw(args(version))] SparsePauliObservableElemPack),
 }
 
 // The V16 version of the Pauli data pack only allows SparsePauliOp and doesn't use a distinguishing first byte
@@ -539,13 +545,13 @@ pub enum PauliDataPackV16 {
 
 #[binrw]
 #[derive(Debug)]
-#[br(import(version: u8))]
+#[brw(import(version: u8))]
 pub enum PauliDataPack {
     #[br(pre_assert(version <= 16))]
     V16(PauliDataPackV16),
 
     #[br(pre_assert(version >= 17))]
-    V17(PauliDataPackV17),
+    V17(#[brw(args(version))] PauliDataPackV17),
 }
 
 // SparsePauliOpList is a serialized python numpy array
@@ -559,26 +565,76 @@ pub struct SparsePauliOpListElemPack {
     pub data: Bytes,
 }
 
+/// Read the bit terms of a `SPARSE_OBSERVABLE` payload.
+///
+/// `BitTerm` is `#[repr(u8)]`, so a single byte is always enough.  QPY 17 nonetheless stored each
+/// bit term as a `u16`, wasting a byte per term; QPY 18 narrowed it to `u8`.  The in-memory
+/// representation is always `u8` and this parser absorbs the difference, so nothing downstream has
+/// to care which version produced the payload.
+#[binrw::parser(reader, endian)]
+fn read_bitterms(version: u8, byte_count: u64) -> BinResult<Vec<u8>> {
+    let count = (byte_count / bitterm_size(version) as u64) as usize;
+    if version >= 18 {
+        Vec::<u8>::read_options(reader, endian, binrw::VecArgs { count, inner: () })
+    } else {
+        let wide = Vec::<u16>::read_options(reader, endian, binrw::VecArgs { count, inner: () })?;
+        let pos = reader.stream_position().unwrap_or(0);
+        wide.into_iter()
+            .map(|term| {
+                u8::try_from(term).map_err(|_| binrw::Error::AssertFail {
+                    pos,
+                    message: format!("bit term {term} does not fit in a u8"),
+                })
+            })
+            .collect()
+    }
+}
+
+/// Write the bit terms of a `SPARSE_OBSERVABLE` payload, widening back to `u16` for QPY < 18.
+/// Mirror of [`read_bitterms`].
+#[binrw::writer(writer, endian)]
+fn write_bitterms(bitterms: &Vec<u8>, version: u8) -> BinResult<()> {
+    if version >= 18 {
+        bitterms.write_options(writer, endian, ())
+    } else {
+        let wide: Vec<u16> = bitterms.iter().map(|&term| term as u16).collect();
+        wide.write_options(writer, endian, ())
+    }
+}
+
+const fn bitterm_size(version: u8) -> usize {
+    if version <= 17 {
+        std::mem::size_of::<u16>()
+    } else {
+        std::mem::size_of::<u8>()
+    }
+}
+
 // SparsePauiObservable has explicit data that can be used to reconstruct
 // a rust SparseObservable struct
+//
+// Note that the `*_size` fields are element *counts*, not byte lengths.
 #[binrw]
 #[brw(big)]
 #[derive(Debug)]
+#[brw(import(version: u8))]
 pub struct SparsePauliObservableElemPack {
     pub num_qubits: u32,
     // coeffs are Complex64 numbers, stored as a vector of f64 in the format [re1, im1, re2, im2,...]
     #[bw(calc = (coeff_data.len() * std::mem::size_of::<f64>()) as u64)]
     pub coeff_data_size: u64,
-    #[bw(calc = (bitterm_data.len() * std::mem::size_of::<u16>()) as u64)]
+    #[bw(calc = (bitterm_data.len() * bitterm_size(version)) as u64)]
     pub bitterm_data_size: u64,
     #[bw(calc = (inds_data.len() * std::mem::size_of::<u32>()) as u64)]
     pub inds_data_size: u64,
     #[bw(calc = (bounds_data.len() * std::mem::size_of::<u64>()) as u64)]
     pub bounds_data_size: u64,
     #[br(count = coeff_data_size / std::mem::size_of::<f64>() as u64)]
     pub coeff_data: Vec<f64>, // complex numbers stored in format [re1, im1, re2, im2,...]
-    #[br(count = bitterm_data_size / std::mem::size_of::<u16>() as u64)]
-    pub bitterm_data: Vec<u16>,
+    // Stored as `u16` up to QPY 17 and as `u8` from QPY 18 on; always `u8` in memory.
+    #[br(parse_with = read_bitterms, args(version, bitterm_data_size))]
+    #[bw(write_with = write_bitterms, args(version))]
+    pub bitterm_data: Vec<u8>,
     #[br(count = inds_data_size / std::mem::size_of::<u32>() as u64)]
     pub inds_data: Vec<u32>,
     #[br(count = bounds_data_size / std::mem::size_of::<u64>() as u64)]

crates/qpy/src/py_methods.rs

@@ -203,7 +203,7 @@ fn pack_sparse_pauli_op(
         let bitterm_data = sparse_observable
             .bit_terms()
             .iter()
-            .map(|&bitterm| bitterm as u16)
+            .map(|&bitterm| bitterm as u8)
             .collect();
         let inds_data = sparse_observable.indices().to_vec();
         let bounds_data = sparse_observable

qiskit/qpy/__init__.py

@@ -512,6 +512,15 @@ def open(*args):
 payload: ``conditional_reg_name_size`` for a condition, and the ``INSTRUCTION_PARAM`` header's
 ``size`` for a parameter.
 
+Version 18 also narrows the bit-term elements of the `SPARSE_OBSERVABLE` payload from `"!H"`
+(``uint16_t``) to `"!B"` (``uint8_t``).
+
+The values of :class:`.SparseObservable.BitTerm` always fit in a single byte, so the wider type
+stored a byte of padding for every bit term.  A version 18 payload is therefore one byte smaller
+per bit term than the equivalent version 17 payload.  No other field of `SPARSE_OBSERVABLE`
+changes, and the meaning of `bitterm_data_len` is unaffected because it counts elements rather
+than bytes.
+
 Changes to REGISTER_PACK
 ~~~~~~~~~~~~~~~~~~~~~~~~
 
@@ -583,14 +592,15 @@ def open(*args):
   }
 
 which is immediately followed by the number of qubits and then the data arrays of the
-coefficients, bit terms, indices, and boundaries of the observable. The format specifies the
-number of bytes each array occupies. The number of elements can be calculated by dividing
-the number of bytes by the size of each element.
+coefficients, bit terms, indices, and boundaries of the observable. Each of the four ``*_len``
+fields is the number of **elements** in the corresponding array, not the number of bytes it
+occupies; multiply by the size of the element type to get the byte length.
 
  * Each coefficient is stored as two consecutive `"!d"` elements, first the real and then
-   the imaginary part.
- * The bit term elements are of type `"!H"` and represents the `u8` value of the
-   :class:`.SparseObservable.BitTerm`
+   the imaginary part, so ``coeff_data_len`` is twice the number of coefficients.
+ * The bit term elements are of type `"!H"` and represent the `u8` value of the
+   :class:`.SparseObservable.BitTerm`.  From :ref:`version 18 <qpy_version_18>` onwards these
+   are stored as `"!B"` instead.
  * The indices elements are of type `"!I"`.
  * The boundaries elements are of type `"!Q"`.
 

releasenotes/notes/qpy-18-bitterm-u8-2d45de9e7c99847a.yaml

@@ -0,0 +1,13 @@
+---
+upgrade_qpy:
+  - |
+    QPY has been upgraded to version 18, which narrows the bit-term elements of the
+    ``SPARSE_OBSERVABLE`` payload from ``uint16_t`` to ``uint8_t``.  The values of
+    :class:`.SparseObservable.BitTerm` always fit in a single byte, so the previous encoding
+    stored a byte of padding for every bit term.  Payloads containing a
+    :class:`.PauliEvolutionGate` whose operators are :class:`.SparseObservable` instances are
+    now one byte smaller per bit term, with no loss of information.
+
+    This change is backwards compatible.  QPY version 17 and earlier payloads load exactly as
+    before, and passing ``version=17`` to :func:`.qpy.dump` still emits the older encoding.
+    See :mod:`qiskit.qpy` for the updated format description.

test/python/qpy/test_v18.py

@@ -17,9 +17,11 @@
 
 from qiskit.circuit import ClassicalRegister, QuantumCircuit, QuantumRegister, Qubit
 from qiskit.circuit.classical import expr
+from qiskit.circuit.library import PauliEvolutionGate
 from qiskit.qpy import dump, load
 from qiskit.qpy import formats
 from qiskit.qpy.exceptions import QpyError
+from qiskit.quantum_info import SparseObservable, SparsePauliOp
 from test import QiskitTestCase
 
 
@@ -172,3 +174,60 @@ def test_v18_unknown_register_name_is_rejected(self):
         data[start + 1 : start + 5] = b"zzzz"  # same length, so no size field to fix up
         with self.assertRaises(QpyError):
             load(io.BytesIO(bytes(data)))
+
+
+class TestV18SparseObservable(QiskitTestCase):
+    """``SPARSE_OBSERVABLE`` payloads, whose bit terms narrowed from ``uint16_t`` to ``uint8_t``.
+
+    QPY gained :class:`.SparseObservable` in v17 and v18 narrowed the stored bit terms, so between
+    them these two versions cover both encodings.
+
+    Only those two are covered, for two independent reasons.  The Rust writer is the only one that
+    emits QPY >= 17 (``QPY_RUST_WRITE_MIN_VERSION``), and a payload written by one implementation
+    cannot currently be read by the other, because the two codecs disagree over whether the
+    ``*_data_len`` fields hold a byte length or an element count.  Separately, asking for a version
+    below 17 does not raise: the writer emits a v17-shaped element into the older payload, which no
+    reader can then parse.
+    """
+
+    # TODO - the cross-implementation half of this is bug #16722; once that is fixed these can also
+    # be covered by the writer/reader matrix in test_roundtrip.py.
+    VERSIONS = (17, 18)
+
+    def _assert_roundtrips(self, circuit):
+        """The circuit survives a dump/load at each version that can express it."""
+        for version in self.VERSIONS:
+            with self.subTest(version=version):
+                self.assertEqual(load(io.BytesIO(_dump(circuit, version)))[0], circuit)
+
+    def test_evolutiongate_sparse_observable(self):
+        """An evolution gate over a SparseObservable round-trips under both bit-term widths.
+
+        The operator uses every :class:`.SparseObservable.BitTerm` variant, so the full value range
+        of that field is exercised.
+        """
+        op = SparseObservable.from_list(
+            [
+                ("XIII", 0.1),
+                ("YIII", 0.2),
+                ("ZIII", 0.3),
+                ("+III", 0.4),
+                ("-III", 0.5),
+                ("rIII", 0.6),
+                ("lIII", 0.7),
+                ("0III", 0.8),
+                ("1III", 0.9),
+            ]
+        )
+        qc = QuantumCircuit(op.num_qubits)
+        qc.append(PauliEvolutionGate(op, time=0.3), qc.qubits)
+        self._assert_roundtrips(qc)
+
+    def test_evolutiongate_mixed_operators(self):
+        """An evolution gate over a list mixing SparseObservable and SparsePauliOp."""
+        op1 = SparseObservable.from_list([("XIX", 0.1), ("ZIZ", 0.3)])
+        op2 = SparsePauliOp.from_list([("ZZI", 1), ("XIX", -0.1)])
+        evo = PauliEvolutionGate([op1, op2], time=0.5)
+        qc = QuantumCircuit(evo.num_qubits)
+        qc.append(evo, qc.qubits)
+        self._assert_roundtrips(qc)