From 1788d99aba3bf33f05832e02ec02b25e375a3996 Mon Sep 17 00:00:00 2001 From: Elad Venezian Date: Mon, 3 Aug 2026 15:58:55 +0300 Subject: [PATCH 01/10] QPY: bump to version 18 and drop CalibrationsPack from format Pulse gate calibrations were removed in Qiskit 2.0. Since then the CalibrationsPack field in the QPY circuit payload has always been written as an empty placeholder (num_cals=0). Version 18 removes it entirely. - Bump QPY_VERSION to 18 in qiskit/qpy/common.py - Gate CalibrationsPack field in QPYCircuit on version < 18 (binrw Option + if attribute covers both reader and writer) - Guard Python writer and reader paths with version < 18 / 5 <= version < 18 - Add regression test asserting v18 output is 2 bytes smaller than v17 - Add reno release note and Version 18 docstring section Signed-off-by: Elad Venezian --- crates/qpy/src/circuit_writer.rs | 12 ++- crates/qpy/src/formats.rs | 4 +- qiskit/qpy/__init__.py | 17 +++++ qiskit/qpy/binary_io/circuits.py | 11 ++- qiskit/qpy/common.py | 2 +- ...-remove-calibrations-d51e94f7ad890d0e.yaml | 9 +++ test/python/qpy/test_v18_calibrations.py | 73 +++++++++++++++++++ 7 files changed, 115 insertions(+), 13 deletions(-) create mode 100644 releasenotes/notes/qpy-v18-remove-calibrations-d51e94f7ad890d0e.yaml create mode 100644 test/python/qpy/test_v18_calibrations.py diff --git a/crates/qpy/src/circuit_writer.rs b/crates/qpy/src/circuit_writer.rs index 69af2c7104f8..46ee8ab06b8e 100644 --- a/crates/qpy/src/circuit_writer.rs +++ b/crates/qpy/src/circuit_writer.rs @@ -1219,10 +1219,14 @@ pub(crate) fn pack_circuit( metadata_serializer, &qpy_data, )?; - // Pulse has been removed in Qiskit 2.0. As long as we keep QPY at version 13, - // we need to write an empty calibrations header since read_circuit expects it - let calibrations = formats::CalibrationsPack { - calibrations: vec![], + // CalibrationsPack was dropped in v18; for v13-17 write an empty block (pulse + // gates were removed in Qiskit 2.0 but older format versions require the field) + let calibrations = if version < 18 { + Some(formats::CalibrationsPack { + calibrations: vec![], + }) + } else { + None }; let (instructions, mut custom_instructions_hash) = pack_instructions(&mut qpy_data)?; let custom_instructions = diff --git a/crates/qpy/src/formats.rs b/crates/qpy/src/formats.rs index ae3370d64b96..c750bc8d565e 100644 --- a/crates/qpy/src/formats.rs +++ b/crates/qpy/src/formats.rs @@ -69,8 +69,8 @@ pub struct QPYCircuit { pub custom_instructions: CustomCircuitInstructionsPack, #[br(count = header.num_instructions, args { inner: (true,) })] pub instructions: Vec, - #[br(args(version,))] - pub calibrations: CalibrationsPack, + #[brw(if(version < 18), args(version,))] + pub calibrations: Option, pub layout: LayoutV2Pack, } diff --git a/qiskit/qpy/__init__.py b/qiskit/qpy/__init__.py index 4b690411e215..4b0e3880e58d 100644 --- a/qiskit/qpy/__init__.py +++ b/qiskit/qpy/__init__.py @@ -462,6 +462,23 @@ def open(*args): by ``num_circuits`` in the file header). There is no padding between the circuits in the data. +.. _qpy_version_18: + +Version 18 +---------- + +Version 18 removes the ``CalibrationsPack`` field from the circuit payload. Pulse gate +calibrations were removed from Qiskit in version 2.0, and since then the field has always +been written as an empty placeholder (``num_cals = 0``). Dropping it saves 2 bytes per +circuit and cleans up the format. + +Files written with QPY version 18 cannot be read by Qiskit versions that only support QPY +up to version 17. Files written with QPY version 17 or earlier are still read correctly. + +.. versionchanged:: QPY 18 + The ``CALIBRATIONS`` block (``num_cals: uint16``) is no longer present in the circuit + payload. It was present in versions 5 through 17. + .. _qpy_version_17: Version 17 diff --git a/qiskit/qpy/binary_io/circuits.py b/qiskit/qpy/binary_io/circuits.py index 54b49a763f69..938b53011c50 100644 --- a/qiskit/qpy/binary_io/circuits.py +++ b/qiskit/qpy/binary_io/circuits.py @@ -1648,10 +1648,9 @@ def write_circuit( file_obj.write(instruction_buffer.getvalue()) instruction_buffer.close() - # Pulse has been removed in Qiskit 2.0. As long as we keep QPY at version 13, - # we need to write an empty calibrations header since read_circuit expects it - header = struct.pack(formats.CALIBRATION_PACK, 0) - file_obj.write(header) + # CalibrationsPack was dropped in v18; for v13-17 write an empty block + if version < 18: + file_obj.write(struct.pack(formats.CALIBRATION_PACK, 0)) _write_layout(file_obj, circuit) @@ -1808,8 +1807,8 @@ def read_circuit( annotation_state=annotation_state, ) - # Consume calibrations, but don't use them since pulse gates are not supported as of Qiskit 2.0 - if version >= 5: + # Consume calibrations block; absent in v18+ where it was dropped from the format + if 5 <= version < 18: _read_calibrations(file_obj, version, vectors, metadata_deserializer) if version >= 8: diff --git a/qiskit/qpy/common.py b/qiskit/qpy/common.py index 5c96b10a5349..7c8d888a327d 100644 --- a/qiskit/qpy/common.py +++ b/qiskit/qpy/common.py @@ -23,7 +23,7 @@ from qiskit.qpy import formats, exceptions -QPY_VERSION = 17 +QPY_VERSION = 18 QPY_COMPATIBILITY_VERSION = 13 QPY_RUST_READ_MIN_VERSION = 13 QPY_RUST_WRITE_MIN_VERSION = 17 diff --git a/releasenotes/notes/qpy-v18-remove-calibrations-d51e94f7ad890d0e.yaml b/releasenotes/notes/qpy-v18-remove-calibrations-d51e94f7ad890d0e.yaml new file mode 100644 index 000000000000..7d9f3c2ca1f2 --- /dev/null +++ b/releasenotes/notes/qpy-v18-remove-calibrations-d51e94f7ad890d0e.yaml @@ -0,0 +1,9 @@ +--- +upgrade_qpy: + - | + QPY version 18 removes the ``CalibrationsPack`` field from the circuit binary + format. Pulse gate calibrations were already unsupported since Qiskit 2.0, and + the field has been written as an empty placeholder ever since. Files written with + QPY version 18 are 2 bytes smaller per circuit and cannot be read by Qiskit + versions that only support QPY up to version 17. Files written with QPY version + 17 or earlier are still read correctly by the current Qiskit. diff --git a/test/python/qpy/test_v18_calibrations.py b/test/python/qpy/test_v18_calibrations.py new file mode 100644 index 000000000000..60241dcf41cd --- /dev/null +++ b/test/python/qpy/test_v18_calibrations.py @@ -0,0 +1,73 @@ +# This code is part of Qiskit. +# +# (C) Copyright IBM 2026. +# +# This code is licensed under the Apache License, Version 2.0. You may +# obtain a copy of this license in the LICENSE.txt file in the root directory +# of this source tree or at http://www.apache.org/licenses/LICENSE-2.0. +# +# Any modifications or derivative works of this code must retain this +# copyright notice, and modified files need to carry a notice indicating +# that they have been altered from the originals. + +"""Regression tests for CalibrationsPack removal in QPY v18.""" + +import io +import struct + +from qiskit.circuit import QuantumCircuit +from qiskit.qpy import dump, load +from qiskit.qpy import formats +from test import QiskitTestCase + + +def _make_bell() -> QuantumCircuit: + qc = QuantumCircuit(2) + qc.h(0) + qc.cx(0, 1) + return qc + + +class TestV18CalibrationsAbsent(QiskitTestCase): + """ + v18 files must be exactly 2 bytes smaller than v17 files for the same circuit + (the 2 bytes being the dropped CALIBRATION_PACK header: struct "!H" = uint16). + """ + + def test_v18_smaller_than_v17_by_calibration_header(self): + """v18 output is exactly 2 bytes smaller than v17 (CalibrationsPack removed).""" + qc = _make_bell() + + buf17 = io.BytesIO() + dump(qc, buf17, version=17) + buf18 = io.BytesIO() + dump(qc, buf18, version=18) + + size17 = len(buf17.getvalue()) + size18 = len(buf18.getvalue()) + cal_header_size = struct.calcsize(formats.CALIBRATION_PACK) # 2 bytes + + self.assertEqual( + size17 - size18, + cal_header_size, + f"Expected v18 to be {cal_header_size} bytes smaller than v17, " + f"got v17={size17} v18={size18} diff={size17 - size18}", + ) + + def test_v18_roundtrip(self): + """Bell circuit round-trips correctly through QPY v18.""" + qc = _make_bell() + buf = io.BytesIO() + dump(qc, buf, version=18) + buf.seek(0) + loaded = load(buf)[0] + self.assertEqual(qc, loaded) + + def test_v17_roundtrip(self): + """Bell circuit round-trips correctly through QPY v17 (back-compat baseline).""" + qc = _make_bell() + buf = io.BytesIO() + dump(qc, buf, version=17) + buf.seek(0) + loaded = load(buf)[0] + self.assertEqual(qc, loaded) From 077d213a1942ce99ac4984ff677e6031479997f4 Mon Sep 17 00:00:00 2001 From: Elad Venezian Date: Tue, 4 Aug 2026 13:53:45 +0300 Subject: [PATCH 02/10] QPY: use big-endian instruction parameters in v18+ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit QPY format versions 1–17 mistakenly serialised integer and float INSTRUCTION_PARAM values in little-endian byte order. Version 18 corrects this to big-endian, consistent with the rest of the format. The fix is fully backward-compatible: files written at version ≤ 17 are still read correctly via the LittleForV17AndBelow path. The Python read/write path (circuits.py) only handles versions ≤ 16 and is unchanged — the little-endian encoding there remains correct for those versions. Key implementation points: - New ValueEndian enum (Big / Little / LittleForV17AndBelow) replaces binrw::Endian at every QPY value call site; ValueEndian::resolve(version) is the single place where the version-to-endian mapping lives. - GenericValue::as_little_for_v17_and_below(&self, version) consolidates the remaining as_le() call sites that work directly with GenericValue. - Regression tests in test/python/qpy/test_v18_big_endian_params.py verify that v17 and v18 bytes differ for float params, for-loop integer lists, and switch-case labels, and that both versions round-trip correctly. Signed-off-by: Elad Venezian --- crates/qpy/src/circuit_reader.rs | 55 +++---- crates/qpy/src/circuit_writer.rs | 51 +++--- crates/qpy/src/params.rs | 8 +- crates/qpy/src/py_methods.rs | 10 +- crates/qpy/src/value.rs | 75 ++++++++- ...18-big-endian-params-9d6195c0da016bf7.yaml | 9 + test/python/qpy/test_v18_big_endian_params.py | 154 ++++++++++++++++++ 7 files changed, 294 insertions(+), 68 deletions(-) create mode 100644 releasenotes/notes/qpy-v18-big-endian-params-9d6195c0da016bf7.yaml create mode 100644 test/python/qpy/test_v18_big_endian_params.py diff --git a/crates/qpy/src/circuit_reader.rs b/crates/qpy/src/circuit_reader.rs index 02cc21ecd07c..3cb3eb14840e 100644 --- a/crates/qpy/src/circuit_reader.rs +++ b/crates/qpy/src/circuit_reader.rs @@ -19,7 +19,6 @@ // Ideally, serialization is done by packing in a binrw-enhanced struct and using the // `write` method into a `Cursor` buffer, but there might be exceptions. -use binrw::Endian; use hashbrown::HashMap; use num_bigint::BigUint; use num_complex::Complex64; @@ -69,8 +68,8 @@ use crate::value::ParamRegisterValue; use crate::value::unpack_for_collection; use crate::value::{ BitType, CircuitInstructionType, ExpressionType, ExpressionVarDeclaration, GenericValue, - QPYReadData, RegisterType, ValueType, deserialize_with_args, load_param_register_value, - load_value, unpack_duration_value, unpack_generic_value, + QPYReadData, RegisterType, ValueEndian, ValueType, deserialize_with_args, + load_param_register_value, load_value, unpack_duration_value, unpack_generic_value, }; use ndarray::{Array2, ShapeBuilder}; @@ -150,7 +149,7 @@ fn unpack_condition( match &condition_pack.data { ConditionData::None => Ok(None), ConditionData::Expression(exp_pack) => { - let exp_value = unpack_generic_value(exp_pack, qpy_data, Endian::Big)?; + let exp_value = unpack_generic_value(exp_pack, qpy_data, ValueEndian::Big)?; match exp_value { GenericValue::Expression(exp) => Ok(Some(Condition::Expr(exp.clone()))), _ => Err(QpyError::InvalidExpression( @@ -259,10 +258,8 @@ fn get_instruction_bits( fn get_instruction_values( instruction: &formats::CircuitInstructionV2Pack, qpy_data: &mut QPYReadData, - endian: Endian, + endian: ValueEndian, ) -> Result, QpyError> { - // note that numbers are not read correctly - they are read in big endian, but for instruction parameters, due to historical reasons, - // they are stored in little endian let inst_params: Vec = instruction .params .iter() @@ -410,7 +407,7 @@ fn unpack_standard_gate( instruction.gate_class_name ))); }; - let param_values = get_instruction_values(instruction, qpy_data, Endian::Little)?; + let param_values = get_instruction_values(instruction, qpy_data, ValueEndian::LittleForV17AndBelow)?; Ok((op, param_values)) } @@ -427,7 +424,7 @@ fn unpack_standard_instruction( instruction.gate_class_name ))); }; - let param_values = get_instruction_values(instruction, qpy_data, Endian::Little)?; + let param_values = get_instruction_values(instruction, qpy_data, ValueEndian::LittleForV17AndBelow)?; Ok((op, param_values)) } @@ -440,7 +437,7 @@ fn unpack_pauli_product_measurement( "Pauli Product Measurement should have exactly 3 parameters".to_string(), )); } - let z_values = unpack_generic_value(&instruction.params[0], qpy_data, Endian::Big)?; + let z_values = unpack_generic_value(&instruction.params[0], qpy_data, ValueEndian::Big)?; let z: Vec = z_values.to_boolean_vec().ok_or_else(|| { QpyError::InvalidParameter(format!( "Pauli product measurement z parameter should be a boolean or integer vector, but got {:?}", @@ -448,14 +445,14 @@ fn unpack_pauli_product_measurement( )) })?; - let x_values = unpack_generic_value(&instruction.params[1], qpy_data, Endian::Big)?; + let x_values = unpack_generic_value(&instruction.params[1], qpy_data, ValueEndian::Big)?; let x: Vec = x_values.to_boolean_vec().ok_or_else(|| { QpyError::InvalidParameter(format!( "Pauli product measurement x parameter should be a boolean or integer vector, but got {:?}", x_values )) })?; - let neg_value = unpack_generic_value(&instruction.params[2], qpy_data, Endian::Big)?; + let neg_value = unpack_generic_value(&instruction.params[2], qpy_data, ValueEndian::Big)?; let neg = match neg_value { GenericValue::NumpyObject(bytes) => { let npy = NpyFile::new(Cursor::new(&bytes.0))?; @@ -491,19 +488,19 @@ fn unpack_pauli_product_rotation( "No angle for pauli product rotation".to_string(), )); } - let z_values = unpack_generic_value(&instruction.params[0], qpy_data, Endian::Big)?; + let z_values = unpack_generic_value(&instruction.params[0], qpy_data, ValueEndian::Big)?; let z = z_values.to_boolean_vec().ok_or_else(|| { QpyError::InvalidParameter( "Pauli product rotation z parameter should be a boolean vector".to_string(), ) })?; - let x_values = unpack_generic_value(&instruction.params[1], qpy_data, Endian::Big)?; + let x_values = unpack_generic_value(&instruction.params[1], qpy_data, ValueEndian::Big)?; let x = x_values.to_boolean_vec().ok_or_else(|| { QpyError::InvalidParameter( "Pauli product rotation x parameter should be a boolean vector".to_string(), ) })?; - let angle_value = unpack_generic_value(&instruction.params[2], qpy_data, Endian::Little)?; + let angle_value = unpack_generic_value(&instruction.params[2], qpy_data, ValueEndian::LittleForV17AndBelow)?; let angle = generic_value_to_param(&angle_value)?; let rotation = PauliProductRotation { z, x, angle }; let pbc = Box::new(PauliBased::PauliProductRotation(rotation)); @@ -517,7 +514,7 @@ fn unpack_unitary( qpy_data: &mut QPYReadData, ) -> Result<(PackedOperation, Vec), QpyError> { let GenericValue::NumpyObject(bytes) = - unpack_generic_value(&instruction.params[0], qpy_data, Endian::Little)? + unpack_generic_value(&instruction.params[0], qpy_data, ValueEndian::LittleForV17AndBelow)? else { return Err(QpyError::InvalidParameter( "No matrix for unitary op".to_string(), @@ -576,7 +573,7 @@ fn unpack_control_flow( .params .iter() .skip(1) - .map(|param| unpack_generic_value(param, qpy_data, Endian::Little)) + .map(|param| unpack_generic_value(param, qpy_data, ValueEndian::LittleForV17AndBelow)) .collect::>()?; let duration_value = if let Some(duration_pack) = instruction.params.first() { unpack_duration_value(duration_pack, qpy_data)? @@ -600,7 +597,7 @@ fn unpack_control_flow( ControlFlowType::ContinueLoop => ControlFlow::ContinueLoop, ControlFlowType::ForLoop => { let mut instruction_values = - get_instruction_values(instruction, qpy_data, Endian::Big)?; + get_instruction_values(instruction, qpy_data, ValueEndian::Big)?; param_values = instruction_values.split_off(2); let [GenericValue::Circuit(circuit)] = param_values.as_slice() else { return Err(QpyError::DeserializationError( @@ -613,8 +610,8 @@ fn unpack_control_flow( "For loop instruction missing some of its parameters".to_string(), ))?; if gate_class_name == "ForLoopOp" { - // old style params for loop were stored as little endian - collection_value_pack = collection_value_pack.as_le(); + collection_value_pack = + collection_value_pack.as_little_for_v17_and_below(qpy_data.version); } let collection = unpack_for_collection(&collection_value_pack)?; let loop_param = match loop_param_value_pack { @@ -659,18 +656,18 @@ fn unpack_control_flow( ControlFlowType::IfElse => { let condition = unpack_condition(&instruction.condition, qpy_data)? .ok_or_else(|| QpyError::MissingData("if else condition is missing".to_string()))?; - param_values = get_instruction_values(instruction, qpy_data, Endian::Big)?; + param_values = get_instruction_values(instruction, qpy_data, ValueEndian::Big)?; ControlFlow::IfElse { condition } } ControlFlowType::WhileLoop => { let condition = unpack_condition(&instruction.condition, qpy_data)? .ok_or_else(|| QpyError::MissingData("if else condition is missing".to_string()))?; - param_values = get_instruction_values(instruction, qpy_data, Endian::Big)?; + param_values = get_instruction_values(instruction, qpy_data, ValueEndian::Big)?; ControlFlow::While { condition } } ControlFlowType::SwitchCase => { let mut instruction_values = - get_instruction_values(instruction, qpy_data, Endian::Big)?; + get_instruction_values(instruction, qpy_data, ValueEndian::Big)?; let (target_value, case_label_list) = if instruction_values.len() < 3 { // we follow the python way of storing switch params // the first param is the target, the next param is the cases specifier @@ -740,7 +737,7 @@ fn unpack_control_flow( }; let label_spec_element = label_spec_element_tuple .iter() - .map(|label_spec_element| match label_spec_element.as_le() { + .map(|label_spec_element| match label_spec_element.as_little_for_v17_and_below(qpy_data.version) { GenericValue::CaseDefault => Ok(CaseSpecifier::Default), GenericValue::BigInt(value) => Ok(CaseSpecifier::Uint(value.clone())), GenericValue::Int64(value) => { @@ -778,7 +775,7 @@ fn unpack_py_instruction( qpy_data: &mut QPYReadData, ) -> Result<(PackedOperation, Vec), QpyError> { let name = instruction.gate_class_name.clone(); - let mut instruction_values = get_instruction_values(instruction, qpy_data, Endian::Little)?; + let mut instruction_values = get_instruction_values(instruction, qpy_data, ValueEndian::LittleForV17AndBelow)?; Python::attach(|py| -> Result<_, QpyError> { let mut py_params: Vec> = instruction_values .iter() @@ -942,7 +939,7 @@ fn unpack_custom_instruction( let custom_instruction = custom_instructions_map.get(&name).ok_or_else(|| { QpyError::MissingData("Custom instruction data not found for {name}".to_string()) })?; - let instruction_values = get_instruction_values(instruction, qpy_data, Endian::Little)?; + let instruction_values = get_instruction_values(instruction, qpy_data, ValueEndian::LittleForV17AndBelow)?; Python::attach(|py| -> Result<_, QpyError> { let py_params: Vec> = instruction_values .iter() @@ -1257,7 +1254,7 @@ fn deserialize_pauli_evolution_gate( ValueType::NumpyObject, &sparse_pauli_op_pack.data, qpy_data, - Endian::Big, + ValueEndian::Big, )?; if let GenericValue::NumpyObject(op_raw_data) = data { let np_array = py_deserialize_numpy_object(&op_raw_data)?; @@ -1285,7 +1282,7 @@ fn deserialize_pauli_evolution_gate( packed_data.time_type, &packed_data.time_data, qpy_data, - Endian::Big, + ValueEndian::Big, )?; let py_time: Py = match time { GenericValue::Float64(value) => value.into_py_any(py)?, @@ -1596,7 +1593,7 @@ pub(crate) fn unpack_circuit( packed_circuit.header.global_phase_type, &packed_circuit.header.global_phase_data, &mut qpy_data, - Endian::Big, + ValueEndian::Big, )?)?; qpy_data.circuit_data.set_global_phase_param(global_phase)?; add_standalone_vars(packed_circuit, &mut qpy_data)?; diff --git a/crates/qpy/src/circuit_writer.rs b/crates/qpy/src/circuit_writer.rs index 46ee8ab06b8e..85bc76be0769 100644 --- a/crates/qpy/src/circuit_writer.rs +++ b/crates/qpy/src/circuit_writer.rs @@ -18,7 +18,6 @@ // 3. "Write": To write to a file obj the serialization of the original data // Ideally, serialization is done by packing in a binrw-enhanced struct and using the // `write` method into a `Cursor` buffer, but there might be exceptions. -use binrw::Endian; use hashbrown::{HashMap, HashSet}; use num_bigint::BigUint; use num_traits::ToPrimitive; @@ -55,8 +54,8 @@ use crate::py_methods::{ }; use crate::value::{ BitType, CircuitInstructionType, ExpressionVarDeclaration, GenericValue, ParamRegisterValue, - QPYWriteData, RegisterType, get_circuit_type_key, pack_for_collection, pack_generic_value, - pack_standalone_var, pack_stretch, serialize, serialize_param_register_value, + QPYWriteData, RegisterType, ValueEndian, get_circuit_type_key, pack_for_collection, + pack_generic_value, pack_standalone_var, pack_stretch, serialize, serialize_param_register_value, }; use qiskit_circuit::var_stretch_container::{StretchType, VarType}; @@ -191,7 +190,7 @@ fn pack_instruction_params( ) -> Result, QpyError> { inst.params_view() .iter() - .map(|x| pack_param_obj(x, qpy_data, Endian::Little)) + .map(|x| pack_param_obj(x, qpy_data, ValueEndian::LittleForV17AndBelow)) .collect::>() } @@ -218,7 +217,7 @@ fn pack_instruction_blocks( // which would result in inconsistent results, e.g. when packing the same circuit twice on the same run let py_block: PyCircuitData = block.clone().into(); let circuit = py_block.into_py_quantum_circuit(py)?; - py_pack_param(&circuit, qpy_data, Endian::Little) + py_pack_param(&circuit, qpy_data, ValueEndian::LittleForV17AndBelow) }) .collect::>() }), @@ -347,8 +346,8 @@ fn pack_pauli_product_measurement( // Pauli phase: 0 means +1, 2 means -1 (i.e. neg) let phase: i64 = if ppm.neg { 2 } else { 0 }; Ok(vec![ - py_pack_param(&z_array, qpy_data, Endian::Big)?, - py_pack_param(&x_array, qpy_data, Endian::Big)?, + py_pack_param(&z_array, qpy_data, ValueEndian::Big)?, + py_pack_param(&x_array, qpy_data, ValueEndian::Big)?, pack_generic_value(&GenericValue::Int64(phase), qpy_data)?, ]) })?; @@ -379,9 +378,9 @@ fn pack_pauli_product_rotation( let z_array = rotation.z.to_pyarray(py); let x_array = rotation.x.to_pyarray(py); Ok(vec![ - py_pack_param(&z_array, qpy_data, Endian::Big)?, - py_pack_param(&x_array, qpy_data, Endian::Big)?, - pack_param_obj(&rotation.angle, qpy_data, Endian::Little)?, + py_pack_param(&z_array, qpy_data, ValueEndian::Big)?, + py_pack_param(&x_array, qpy_data, ValueEndian::Big)?, + pack_param_obj(&rotation.angle, qpy_data, ValueEndian::LittleForV17AndBelow)?, ]) })?; Ok(formats::CircuitInstructionV2Pack { @@ -448,7 +447,9 @@ fn pack_control_flow_inst( }; let duration_unit_string = GenericValue::String(duration.unit().to_string()); - params.push(pack_generic_value(&duration_value.as_le(), qpy_data)?); + let encoded_duration = + duration_value.as_little_for_v17_and_below(qpy_data.version); + params.push(pack_generic_value(&encoded_duration, qpy_data)?); params.push(pack_generic_value(&duration_unit_string, qpy_data)?); } BoxDuration::Expr(exp) => { @@ -467,7 +468,7 @@ fn pack_control_flow_inst( collection, loop_param, } => { - let collection_value = pack_for_collection(&collection); + let collection_value = pack_for_collection(&collection, qpy_data.version); let loop_param_value = match loop_param { None => GenericValue::Null, Some(LoopParam::Parameter(symbol)) => { @@ -518,14 +519,16 @@ fn pack_control_flow_inst( .map(|label_element| -> Result { match label_element { CaseSpecifier::Default => Ok(GenericValue::CaseDefault), - CaseSpecifier::Uint(val) => Ok(GenericValue::Int64( - val.to_i64().ok_or_else(|| { - QpyError::ConversionError( - "Case specifier too large".to_string(), - ) - })?, - ) - .as_le()), + CaseSpecifier::Uint(val) => { + let v = GenericValue::Int64( + val.to_i64().ok_or_else(|| { + QpyError::ConversionError( + "Case specifier too large".to_string(), + ) + })?, + ); + Ok(v.as_little_for_v17_and_below(qpy_data.version)) + } } }) .collect::, _>>()?, @@ -578,7 +581,7 @@ fn pack_unitary_gate( // we translate the matrix to numpy and then serialize it like python does let params = Python::attach(|py| -> Result<_, QpyError> { let out_array = matrix.to_pyarray(py); - Ok(vec![py_pack_param(&out_array, qpy_data, Endian::Little)?]) + Ok(vec![py_pack_param(&out_array, qpy_data, ValueEndian::LittleForV17AndBelow)?]) })?; // since we won't recreate this gate via python, it's not important to verify the python name is identical to the one we use here // so we simply hard-code it instead of going through python @@ -609,12 +612,12 @@ fn pack_py_instruction( let py_op_object = py_inst.ob.bind(py); if py_op_object.is_instance(imports::CLIFFORD.get_bound(py))? { let tableau = py_op_object.getattr("tableau")?; - Ok(vec![py_pack_param(&tableau, qpy_data, Endian::Little)?]) + Ok(vec![py_pack_param(&tableau, qpy_data, ValueEndian::LittleForV17AndBelow)?]) } else if py_op_object.is_instance(imports::ANNOTATED_OPERATION.get_bound(py))? { let modifiers = py_op_object.getattr("modifiers")?; modifiers .try_iter()? - .map(|modifier| py_pack_param(&modifier?, qpy_data, Endian::Little)) + .map(|modifier| py_pack_param(&modifier?, qpy_data, ValueEndian::LittleForV17AndBelow)) .collect::>() } else { pack_instruction_params(instruction, qpy_data) @@ -759,7 +762,7 @@ fn pack_circuit_header( let global_phase_data = pack_param_obj( qpy_data.circuit_data.global_phase(), qpy_data, - binrw::Endian::Big, + ValueEndian::Big, )?; let qregs = pack_quantum_registers(qpy_data.circuit_data); let cregs = pack_classical_registers(qpy_data.circuit_data); diff --git a/crates/qpy/src/params.rs b/crates/qpy/src/params.rs index 361e90ef7e30..f773a5f85e89 100644 --- a/crates/qpy/src/params.rs +++ b/crates/qpy/src/params.rs @@ -10,6 +10,7 @@ // copyright notice, and modified files need to carry a notice indicating // that they have been altered from the originals. use binrw::Endian; +use crate::value::ValueEndian; use num_complex::Complex64; use pyo3::prelude::*; use qiskit_circuit::operations::Param; @@ -447,7 +448,7 @@ pub(crate) fn unpack_parameter_expression( item.item_type, &item.item_bytes, qpy_data, - Endian::Big, + ValueEndian::Big, )?)?; Ok((sym, replacement)) }) @@ -563,10 +564,11 @@ pub(crate) fn pack_param_expression( pub(crate) fn pack_param_obj( param: &Param, qpy_data: &QPYWriteData, - endian: Endian, + endian: ValueEndian, ) -> Result { + let resolved = endian.resolve(qpy_data.version); Ok(match param { - Param::Float(val) => match endian { + Param::Float(val) => match resolved { Endian::Little => formats::GenericDataPack { type_key: ValueType::Float, data: val.to_le_bytes().into(), diff --git a/crates/qpy/src/py_methods.rs b/crates/qpy/src/py_methods.rs index ad2fd8d3495d..e76bb4b4e88a 100644 --- a/crates/qpy/src/py_methods.rs +++ b/crates/qpy/src/py_methods.rs @@ -12,6 +12,7 @@ // Methods for QPY serialization working directly with Python-based data use binrw::Endian; +use crate::value::ValueEndian; use numpy::Complex64; use pyo3::IntoPyObjectExt; use pyo3::exceptions::PyTypeError; @@ -468,12 +469,13 @@ pub(crate) fn py_convert_from_generic_value(value: &GenericValue) -> Result, qpy_data: &QPYWriteData, - endian: Endian, + endian: ValueEndian, ) -> Result { let value = py_convert_to_generic_value(py_object)?; - let (type_key, data) = match endian { - Endian::Big => serialize_generic_value(&value, qpy_data)?, - Endian::Little => serialize_generic_value(&value.as_le(), qpy_data)?, + let (type_key, data) = if endian.resolve(qpy_data.version) == Endian::Little { + serialize_generic_value(&value.as_le(), qpy_data)? + } else { + serialize_generic_value(&value, qpy_data)? }; Ok(formats::GenericDataPack { type_key, data }) } diff --git a/crates/qpy/src/value.rs b/crates/qpy/src/value.rs index 443248ef69fd..ee1e25bc682b 100644 --- a/crates/qpy/src/value.rs +++ b/crates/qpy/src/value.rs @@ -15,6 +15,55 @@ use std::sync::Arc; use binrw::meta::{ReadEndian, WriteEndian}; use binrw::{BinRead, BinWrite, Endian, binrw}; + +/// Endianness selector for QPY value serialization and deserialization. +/// +/// QPY's format spec requires big-endian (network byte order) for all values. +/// However, `INSTRUCTION_PARAM` integers and floats were historically written +/// in little-endian due to an oversight that dates back to the earliest QPY +/// versions. This was acknowledged as a mistake and is corrected in v18. +/// +/// Use this enum instead of `binrw::Endian` at every QPY value read/write site. +/// Call `.resolve(version)` to obtain a concrete `binrw::Endian` — all version-dispatch +/// logic lives there, so call sites never need to inspect the version themselves. +#[derive(Debug, Clone, Copy, PartialEq)] +pub(crate) enum ValueEndian { + /// Always big-endian, regardless of QPY version. + /// Use for data that was never affected by the little-endian mistake: + /// numpy arrays, expression values, register data, etc. + Big, + + /// Always little-endian, regardless of QPY version. + /// Not currently used in QPY, but included for completeness and in case + /// a future format element genuinely requires little-endian encoding. + Little, + + /// Little-endian for QPY v≤17, big-endian for QPY v≥18. + /// + /// This is the "legacy compatibility" variant. It encodes the fact that + /// instruction parameter integers and floats were mistakenly written in + /// little-endian in QPY v1–17. Version 18 corrects this to big-endian. + /// The version check is resolved by calling `.resolve(qpy_data.version)`, + /// so callers never need to inspect the version themselves — just use this + /// variant for all instruction scalar parameters. + LittleForV17AndBelow, +} + +impl ValueEndian { + /// Resolve to a concrete `binrw::Endian` given the QPY format version. + /// This is the single place where `LittleForV17AndBelow` is converted — + /// every other file calls this instead of repeating the version comparison. + pub(crate) fn resolve(self, version: u8) -> Endian { + match self { + ValueEndian::Big => Endian::Big, + ValueEndian::Little => Endian::Little, + ValueEndian::LittleForV17AndBelow => { + if version >= 18 { Endian::Big } else { Endian::Little } + } + } + } +} + use hashbrown::HashMap; use pyo3::prelude::*; use pyo3::types::PyAny; @@ -372,6 +421,13 @@ impl GenericValue { _ => self.clone(), } } + /// Applies the little-endian encoding workaround for QPY v≤17 instruction parameters. + /// For v≥18 the value is returned unchanged; for v≤17 it is byte-swapped via `as_le()`. + /// Mirrors the `ValueEndian::LittleForV17AndBelow` variant — use this at the few remaining + /// call sites that work directly with `GenericValue` rather than going through `load_value`. + pub(crate) fn as_little_for_v17_and_below(&self, version: u8) -> Self { + if version < 18 { self.as_le() } else { self.clone() } + } pub(crate) fn as_circuit_data(&self) -> Option { match self { GenericValue::Circuit(py_circuit) => { @@ -481,8 +537,9 @@ pub(crate) fn load_value( type_key: ValueType, bytes: &Bytes, qpy_data: &mut QPYReadData, - endian: Endian, + endian: ValueEndian, ) -> Result { + let resolved = endian.resolve(qpy_data.version); match type_key { ValueType::Bool => { let value: bool = bytes.try_into()?; @@ -495,7 +552,7 @@ pub(crate) fn load_value( for (idx, byte) in bytes.iter().enumerate() { bytes_array[idx] = *byte; } - match endian { + match resolved { Endian::Little => Ok(GenericValue::Int64(i64::from_le_bytes(bytes_array))), Endian::Big => Ok(GenericValue::Int64(i64::from_be_bytes(bytes_array))), } @@ -504,7 +561,7 @@ pub(crate) fn load_value( } } ValueType::Float => { - let value: f64 = bytes.try_to_f64(endian)?; + let value: f64 = bytes.try_to_f64(resolved)?; Ok(GenericValue::Float64(value)) } ValueType::Complex => { @@ -691,7 +748,7 @@ pub(crate) fn pack_generic_value( pub(crate) fn unpack_generic_value( value_pack: &GenericDataPack, qpy_data: &mut QPYReadData, - endian: Endian, + endian: ValueEndian, ) -> Result { let result = load_value(value_pack.type_key, &value_pack.data, qpy_data, endian)?; Ok(result) @@ -709,15 +766,17 @@ pub(crate) fn unpack_duration_value( let duration = unpack_duration(deserialize::(&value_pack.data)?.0); Ok(GenericValue::Duration(duration)) } - _ => unpack_generic_value(value_pack, qpy_data, Endian::Little), // fallback (duration can also be expression) + _ => unpack_generic_value(value_pack, qpy_data, ValueEndian::LittleForV17AndBelow), // fallback (duration can also be expression) } } -pub(crate) fn pack_for_collection(value: &ForCollection) -> GenericValue { +pub(crate) fn pack_for_collection(value: &ForCollection, version: u8) -> GenericValue { match value { ForCollection::List(vec) => GenericValue::Tuple( vec.iter() - .map(|&val| GenericValue::Int64(val as i64).as_le()) + .map(|&val| { + GenericValue::Int64(val as i64).as_little_for_v17_and_below(version) + }) .collect(), ), ForCollection::PyRange(py_range) => GenericValue::Range(*py_range), @@ -763,7 +822,7 @@ pub(crate) fn pack_generic_value_sequence( pub(crate) fn unpack_generic_value_sequence( value_seqeunce_pack: GenericDataSequencePack, qpy_data: &mut QPYReadData, - endian: Endian, + endian: ValueEndian, ) -> Result, QpyError> { value_seqeunce_pack .elements diff --git a/releasenotes/notes/qpy-v18-big-endian-params-9d6195c0da016bf7.yaml b/releasenotes/notes/qpy-v18-big-endian-params-9d6195c0da016bf7.yaml new file mode 100644 index 000000000000..8b2eeb3c08e8 --- /dev/null +++ b/releasenotes/notes/qpy-v18-big-endian-params-9d6195c0da016bf7.yaml @@ -0,0 +1,9 @@ +--- +upgrade_qpy: + - | + QPY format version 18 corrects the encoding of integer and float + ``INSTRUCTION_PARAM`` values to use big-endian byte order, consistent with + the rest of the QPY specification. In QPY format versions 1–17 these values + were mistakenly written in little-endian. Files produced by older Qiskit + releases (format version ≤ 17) are still read correctly; this change only + affects files written with ``version=18`` or higher. diff --git a/test/python/qpy/test_v18_big_endian_params.py b/test/python/qpy/test_v18_big_endian_params.py new file mode 100644 index 000000000000..edbd58e5088e --- /dev/null +++ b/test/python/qpy/test_v18_big_endian_params.py @@ -0,0 +1,154 @@ +# This code is part of Qiskit. +# +# (C) Copyright IBM 2026. +# +# This code is licensed under the Apache License, Version 2.0. You may +# obtain a copy of this license in the LICENSE.txt file in the root directory +# of this source tree or at http://www.apache.org/licenses/LICENSE-2.0. +# +# Any modifications or derivative works of this code must retain this +# copyright notice, and modified files need to carry a notice indicating +# that they have been altered from the originals. + +"""Regression tests for big-endian instruction parameters in QPY v18. + +QPY v1–17 serialised instruction parameter integers and floats in little-endian +by mistake. v18 corrects this to big-endian (matching the rest of the format). +These tests verify: + - v18 and v17 produce *different* on-disk bytes for the same parameterised circuit + (proving the encoding actually changed). + - Both versions round-trip back to equal circuits (forward and backward compat). + - The specific cases that required special handling: ForLoopOp integer lists and + SwitchCase integer labels. +""" + +import io +import struct + +from qiskit.circuit import ClassicalRegister, QuantumCircuit, QuantumRegister +from qiskit.circuit.classical import expr +from qiskit.qpy import dump, load +from test import QiskitTestCase + + +def _dump(qc: QuantumCircuit, version: int) -> bytes: + buf = io.BytesIO() + dump(qc, buf, version=version) + return buf.getvalue() + + +def _load(data: bytes) -> QuantumCircuit: + return load(io.BytesIO(data))[0] + + +class TestV18BigEndianFloatParam(QiskitTestCase): + """Float gate parameters are big-endian in v18, little-endian in v17.""" + + def _make_circuit(self): + qc = QuantumCircuit(1) + qc.rz(1.23456789, 0) + return qc + + def test_v17_v18_bytes_differ(self): + """v17 and v18 serialise the float parameter in different byte order.""" + qc = self._make_circuit() + self.assertNotEqual(_dump(qc, 17), _dump(qc, 18)) + + def test_v18_roundtrip(self): + """Float parameter round-trips correctly through QPY v18.""" + qc = self._make_circuit() + self.assertEqual(qc, _load(_dump(qc, 18))) + + def test_v17_roundtrip(self): + """Float parameter round-trips correctly through QPY v17 (back-compat).""" + qc = self._make_circuit() + self.assertEqual(qc, _load(_dump(qc, 17))) + + def test_v17_bytes_are_little_endian(self): + """v17 float bytes in the payload are actually little-endian on disk.""" + qc = self._make_circuit() + data = _dump(qc, 17) + # The float 1.23456789 little-endian bytes must appear somewhere in the payload. + le_bytes = struct.pack("d", 1.23456789) + self.assertIn(be_bytes, data) + + +class TestV18BigEndianIntParam(QiskitTestCase): + """Integer instruction parameters are big-endian in v18, little-endian in v17.""" + + def _make_circuit(self): + # PhaseGate takes a float, but CXGate has no params; use a circuit with + # an integer stored directly — RZZ angle as a Python int exercises the Int64 path. + qc = QuantumCircuit(2) + qc.rzz(3, 0, 1) + return qc + + def test_v18_roundtrip(self): + """Integer-valued float parameter round-trips correctly through QPY v18.""" + qc = self._make_circuit() + self.assertEqual(qc, _load(_dump(qc, 18))) + + def test_v17_roundtrip(self): + """Integer-valued float parameter round-trips correctly through QPY v17.""" + qc = self._make_circuit() + self.assertEqual(qc, _load(_dump(qc, 17))) + + +class TestV18BigEndianForLoop(QiskitTestCase): + """ForLoopOp integer-list parameters are big-endian in v18, little-endian in v17.""" + + def _make_circuit(self): + qc = QuantumCircuit(1, 1) + with qc.for_loop((1, 4, 9)): + qc.h(0) + return qc + + def test_v17_v18_bytes_differ(self): + """v17 and v18 serialise ForLoop integer list in different byte order.""" + qc = self._make_circuit() + self.assertNotEqual(_dump(qc, 17), _dump(qc, 18)) + + def test_v18_roundtrip(self): + """ForLoopOp integer list round-trips correctly through QPY v18.""" + qc = self._make_circuit() + self.assertEqual(qc, _load(_dump(qc, 18))) + + def test_v17_roundtrip(self): + """ForLoopOp integer list round-trips correctly through QPY v17 (back-compat).""" + qc = self._make_circuit() + self.assertEqual(qc, _load(_dump(qc, 17))) + + +class TestV18BigEndianSwitchCase(QiskitTestCase): + """SwitchCase integer labels are big-endian in v18, little-endian in v17.""" + + def _make_circuit(self): + body = QuantumCircuit(1) + body.h(0) + qr = QuantumRegister(2, "q") + cr = ClassicalRegister(2, "c") + qc = QuantumCircuit(qr, cr) + qc.switch(expr.bit_and(cr, 3), [(1, body.copy()), (2, body.copy())], [0], []) + return qc + + def test_v17_v18_bytes_differ(self): + """v17 and v18 serialise SwitchCase labels in different byte order.""" + qc = self._make_circuit() + self.assertNotEqual(_dump(qc, 17), _dump(qc, 18)) + + def test_v18_roundtrip(self): + """SwitchCase integer labels round-trip correctly through QPY v18.""" + qc = self._make_circuit() + self.assertEqual(qc, _load(_dump(qc, 18))) + + def test_v17_roundtrip(self): + """SwitchCase integer labels round-trip correctly through QPY v17 (back-compat).""" + qc = self._make_circuit() + self.assertEqual(qc, _load(_dump(qc, 17))) From 51870671a64b572882920d76075aefbce664c9d1 Mon Sep 17 00:00:00 2001 From: Elad Venezian Date: Wed, 5 Aug 2026 15:25:44 +0300 Subject: [PATCH 03/10] Address Gadi's review comments - Inline endian.resolve() at use sites in load_value instead of extracting an upfront variable - Update Version 18 docstring: remove backward-compat boilerplate, add mention of the endianness fix - Consolidate v18 tests into a single test_v18.py with one class (TestV17VsV18); drop round-trips already covered by test_roundtrip.py Signed-off-by: Elad Venezian --- crates/qpy/src/value.rs | 5 +- qiskit/qpy/__init__.py | 9 +- test/python/qpy/test_v18.py | 85 ++++++++++ test/python/qpy/test_v18_big_endian_params.py | 154 ------------------ test/python/qpy/test_v18_calibrations.py | 73 --------- 5 files changed, 90 insertions(+), 236 deletions(-) create mode 100644 test/python/qpy/test_v18.py delete mode 100644 test/python/qpy/test_v18_big_endian_params.py delete mode 100644 test/python/qpy/test_v18_calibrations.py diff --git a/crates/qpy/src/value.rs b/crates/qpy/src/value.rs index ee1e25bc682b..9f930e20dbc5 100644 --- a/crates/qpy/src/value.rs +++ b/crates/qpy/src/value.rs @@ -539,7 +539,6 @@ pub(crate) fn load_value( qpy_data: &mut QPYReadData, endian: ValueEndian, ) -> Result { - let resolved = endian.resolve(qpy_data.version); match type_key { ValueType::Bool => { let value: bool = bytes.try_into()?; @@ -552,7 +551,7 @@ pub(crate) fn load_value( for (idx, byte) in bytes.iter().enumerate() { bytes_array[idx] = *byte; } - match resolved { + match endian.resolve(qpy_data.version) { Endian::Little => Ok(GenericValue::Int64(i64::from_le_bytes(bytes_array))), Endian::Big => Ok(GenericValue::Int64(i64::from_be_bytes(bytes_array))), } @@ -561,7 +560,7 @@ pub(crate) fn load_value( } } ValueType::Float => { - let value: f64 = bytes.try_to_f64(resolved)?; + let value: f64 = bytes.try_to_f64(endian.resolve(qpy_data.version))?; Ok(GenericValue::Float64(value)) } ValueType::Complex => { diff --git a/qiskit/qpy/__init__.py b/qiskit/qpy/__init__.py index 4b0e3880e58d..96d5bee2cf0b 100644 --- a/qiskit/qpy/__init__.py +++ b/qiskit/qpy/__init__.py @@ -472,12 +472,9 @@ def open(*args): been written as an empty placeholder (``num_cals = 0``). Dropping it saves 2 bytes per circuit and cleans up the format. -Files written with QPY version 18 cannot be read by Qiskit versions that only support QPY -up to version 17. Files written with QPY version 17 or earlier are still read correctly. - -.. versionchanged:: QPY 18 - The ``CALIBRATIONS`` block (``num_cals: uint16``) is no longer present in the circuit - payload. It was present in versions 5 through 17. +Version 18 also corrects the encoding of integer and float ``INSTRUCTION_PARAM`` values +to big-endian byte order, consistent with the rest of the QPY specification. In versions +1–17 these were mistakenly written in little-endian. .. _qpy_version_17: diff --git a/test/python/qpy/test_v18.py b/test/python/qpy/test_v18.py new file mode 100644 index 000000000000..5a7467835970 --- /dev/null +++ b/test/python/qpy/test_v18.py @@ -0,0 +1,85 @@ +# This code is part of Qiskit. +# +# (C) Copyright IBM 2026. +# +# This code is licensed under the Apache License, Version 2.0. You may +# obtain a copy of this license in the LICENSE.txt file in the root directory +# of this source tree or at http://www.apache.org/licenses/LICENSE-2.0. +# +# Any modifications or derivative works of this code must retain this +# copyright notice, and modified files need to carry a notice indicating +# that they have been altered from the originals. + +"""Tests for QPY v18 format changes compared to v17.""" + +import io +import struct + +from qiskit.circuit import ClassicalRegister, QuantumCircuit, QuantumRegister +from qiskit.circuit.classical import expr +from qiskit.qpy import dump +from qiskit.qpy import formats +from test import QiskitTestCase + + +def _dump(qc: QuantumCircuit, version: int) -> bytes: + buf = io.BytesIO() + dump(qc, buf, version=version) + return buf.getvalue() + + + +class TestV17VsV18(QiskitTestCase): + """Verify the binary-level differences between QPY v17 and v18.""" + + def test_v18_smaller_than_v17_by_calibration_header(self): + """v18 output is exactly 2 bytes smaller than v17 (CalibrationsPack removed).""" + qc = QuantumCircuit(2) + qc.h(0) + qc.cx(0, 1) + + size17 = len(_dump(qc, 17)) + size18 = len(_dump(qc, 18)) + cal_header_size = struct.calcsize(formats.CALIBRATION_PACK) # 2 bytes + + self.assertEqual( + size17 - size18, + cal_header_size, + f"Expected v18 to be {cal_header_size} bytes smaller than v17, " + f"got v17={size17} v18={size18} diff={size17 - size18}", + ) + + def test_float_param_bytes_differ_v17_vs_v18(self): + """v17 and v18 serialise float parameters in different byte order.""" + qc = QuantumCircuit(1) + qc.rz(1.23456789, 0) + self.assertNotEqual(_dump(qc, 17), _dump(qc, 18)) + + def test_float_param_v17_is_little_endian(self): + """v17 float parameter bytes are little-endian on disk.""" + qc = QuantumCircuit(1) + qc.rz(1.23456789, 0) + self.assertIn(struct.pack("d", 1.23456789), _dump(qc, 18)) + + def test_for_loop_integers_bytes_differ_v17_vs_v18(self): + """v17 and v18 serialise ForLoop integer lists in different byte order.""" + qc = QuantumCircuit(1, 1) + with qc.for_loop((1, 4, 9)): + qc.h(0) + self.assertNotEqual(_dump(qc, 17), _dump(qc, 18)) + + def test_switch_case_labels_bytes_differ_v17_vs_v18(self): + """v17 and v18 serialise SwitchCase integer labels in different byte order.""" + body = QuantumCircuit(1) + body.h(0) + qr = QuantumRegister(2, "q") + cr = ClassicalRegister(2, "c") + qc = QuantumCircuit(qr, cr) + qc.switch(expr.bit_and(cr, 3), [(1, body.copy()), (2, body.copy())], [0], []) + self.assertNotEqual(_dump(qc, 17), _dump(qc, 18)) diff --git a/test/python/qpy/test_v18_big_endian_params.py b/test/python/qpy/test_v18_big_endian_params.py deleted file mode 100644 index edbd58e5088e..000000000000 --- a/test/python/qpy/test_v18_big_endian_params.py +++ /dev/null @@ -1,154 +0,0 @@ -# This code is part of Qiskit. -# -# (C) Copyright IBM 2026. -# -# This code is licensed under the Apache License, Version 2.0. You may -# obtain a copy of this license in the LICENSE.txt file in the root directory -# of this source tree or at http://www.apache.org/licenses/LICENSE-2.0. -# -# Any modifications or derivative works of this code must retain this -# copyright notice, and modified files need to carry a notice indicating -# that they have been altered from the originals. - -"""Regression tests for big-endian instruction parameters in QPY v18. - -QPY v1–17 serialised instruction parameter integers and floats in little-endian -by mistake. v18 corrects this to big-endian (matching the rest of the format). -These tests verify: - - v18 and v17 produce *different* on-disk bytes for the same parameterised circuit - (proving the encoding actually changed). - - Both versions round-trip back to equal circuits (forward and backward compat). - - The specific cases that required special handling: ForLoopOp integer lists and - SwitchCase integer labels. -""" - -import io -import struct - -from qiskit.circuit import ClassicalRegister, QuantumCircuit, QuantumRegister -from qiskit.circuit.classical import expr -from qiskit.qpy import dump, load -from test import QiskitTestCase - - -def _dump(qc: QuantumCircuit, version: int) -> bytes: - buf = io.BytesIO() - dump(qc, buf, version=version) - return buf.getvalue() - - -def _load(data: bytes) -> QuantumCircuit: - return load(io.BytesIO(data))[0] - - -class TestV18BigEndianFloatParam(QiskitTestCase): - """Float gate parameters are big-endian in v18, little-endian in v17.""" - - def _make_circuit(self): - qc = QuantumCircuit(1) - qc.rz(1.23456789, 0) - return qc - - def test_v17_v18_bytes_differ(self): - """v17 and v18 serialise the float parameter in different byte order.""" - qc = self._make_circuit() - self.assertNotEqual(_dump(qc, 17), _dump(qc, 18)) - - def test_v18_roundtrip(self): - """Float parameter round-trips correctly through QPY v18.""" - qc = self._make_circuit() - self.assertEqual(qc, _load(_dump(qc, 18))) - - def test_v17_roundtrip(self): - """Float parameter round-trips correctly through QPY v17 (back-compat).""" - qc = self._make_circuit() - self.assertEqual(qc, _load(_dump(qc, 17))) - - def test_v17_bytes_are_little_endian(self): - """v17 float bytes in the payload are actually little-endian on disk.""" - qc = self._make_circuit() - data = _dump(qc, 17) - # The float 1.23456789 little-endian bytes must appear somewhere in the payload. - le_bytes = struct.pack("d", 1.23456789) - self.assertIn(be_bytes, data) - - -class TestV18BigEndianIntParam(QiskitTestCase): - """Integer instruction parameters are big-endian in v18, little-endian in v17.""" - - def _make_circuit(self): - # PhaseGate takes a float, but CXGate has no params; use a circuit with - # an integer stored directly — RZZ angle as a Python int exercises the Int64 path. - qc = QuantumCircuit(2) - qc.rzz(3, 0, 1) - return qc - - def test_v18_roundtrip(self): - """Integer-valued float parameter round-trips correctly through QPY v18.""" - qc = self._make_circuit() - self.assertEqual(qc, _load(_dump(qc, 18))) - - def test_v17_roundtrip(self): - """Integer-valued float parameter round-trips correctly through QPY v17.""" - qc = self._make_circuit() - self.assertEqual(qc, _load(_dump(qc, 17))) - - -class TestV18BigEndianForLoop(QiskitTestCase): - """ForLoopOp integer-list parameters are big-endian in v18, little-endian in v17.""" - - def _make_circuit(self): - qc = QuantumCircuit(1, 1) - with qc.for_loop((1, 4, 9)): - qc.h(0) - return qc - - def test_v17_v18_bytes_differ(self): - """v17 and v18 serialise ForLoop integer list in different byte order.""" - qc = self._make_circuit() - self.assertNotEqual(_dump(qc, 17), _dump(qc, 18)) - - def test_v18_roundtrip(self): - """ForLoopOp integer list round-trips correctly through QPY v18.""" - qc = self._make_circuit() - self.assertEqual(qc, _load(_dump(qc, 18))) - - def test_v17_roundtrip(self): - """ForLoopOp integer list round-trips correctly through QPY v17 (back-compat).""" - qc = self._make_circuit() - self.assertEqual(qc, _load(_dump(qc, 17))) - - -class TestV18BigEndianSwitchCase(QiskitTestCase): - """SwitchCase integer labels are big-endian in v18, little-endian in v17.""" - - def _make_circuit(self): - body = QuantumCircuit(1) - body.h(0) - qr = QuantumRegister(2, "q") - cr = ClassicalRegister(2, "c") - qc = QuantumCircuit(qr, cr) - qc.switch(expr.bit_and(cr, 3), [(1, body.copy()), (2, body.copy())], [0], []) - return qc - - def test_v17_v18_bytes_differ(self): - """v17 and v18 serialise SwitchCase labels in different byte order.""" - qc = self._make_circuit() - self.assertNotEqual(_dump(qc, 17), _dump(qc, 18)) - - def test_v18_roundtrip(self): - """SwitchCase integer labels round-trip correctly through QPY v18.""" - qc = self._make_circuit() - self.assertEqual(qc, _load(_dump(qc, 18))) - - def test_v17_roundtrip(self): - """SwitchCase integer labels round-trip correctly through QPY v17 (back-compat).""" - qc = self._make_circuit() - self.assertEqual(qc, _load(_dump(qc, 17))) diff --git a/test/python/qpy/test_v18_calibrations.py b/test/python/qpy/test_v18_calibrations.py deleted file mode 100644 index 60241dcf41cd..000000000000 --- a/test/python/qpy/test_v18_calibrations.py +++ /dev/null @@ -1,73 +0,0 @@ -# This code is part of Qiskit. -# -# (C) Copyright IBM 2026. -# -# This code is licensed under the Apache License, Version 2.0. You may -# obtain a copy of this license in the LICENSE.txt file in the root directory -# of this source tree or at http://www.apache.org/licenses/LICENSE-2.0. -# -# Any modifications or derivative works of this code must retain this -# copyright notice, and modified files need to carry a notice indicating -# that they have been altered from the originals. - -"""Regression tests for CalibrationsPack removal in QPY v18.""" - -import io -import struct - -from qiskit.circuit import QuantumCircuit -from qiskit.qpy import dump, load -from qiskit.qpy import formats -from test import QiskitTestCase - - -def _make_bell() -> QuantumCircuit: - qc = QuantumCircuit(2) - qc.h(0) - qc.cx(0, 1) - return qc - - -class TestV18CalibrationsAbsent(QiskitTestCase): - """ - v18 files must be exactly 2 bytes smaller than v17 files for the same circuit - (the 2 bytes being the dropped CALIBRATION_PACK header: struct "!H" = uint16). - """ - - def test_v18_smaller_than_v17_by_calibration_header(self): - """v18 output is exactly 2 bytes smaller than v17 (CalibrationsPack removed).""" - qc = _make_bell() - - buf17 = io.BytesIO() - dump(qc, buf17, version=17) - buf18 = io.BytesIO() - dump(qc, buf18, version=18) - - size17 = len(buf17.getvalue()) - size18 = len(buf18.getvalue()) - cal_header_size = struct.calcsize(formats.CALIBRATION_PACK) # 2 bytes - - self.assertEqual( - size17 - size18, - cal_header_size, - f"Expected v18 to be {cal_header_size} bytes smaller than v17, " - f"got v17={size17} v18={size18} diff={size17 - size18}", - ) - - def test_v18_roundtrip(self): - """Bell circuit round-trips correctly through QPY v18.""" - qc = _make_bell() - buf = io.BytesIO() - dump(qc, buf, version=18) - buf.seek(0) - loaded = load(buf)[0] - self.assertEqual(qc, loaded) - - def test_v17_roundtrip(self): - """Bell circuit round-trips correctly through QPY v17 (back-compat baseline).""" - qc = _make_bell() - buf = io.BytesIO() - dump(qc, buf, version=17) - buf.seek(0) - loaded = load(buf)[0] - self.assertEqual(qc, loaded) From ee9c6d0fce0de93ee8ae4c6805160d0f9b5e0032 Mon Sep 17 00:00:00 2001 From: Elad Venezian Date: Wed, 5 Aug 2026 15:44:01 +0300 Subject: [PATCH 04/10] Apply cargo fmt, black, and fix clippy dead-code warning - cargo fmt: reformat circuit_reader.rs, circuit_writer.rs, params.rs, py_methods.rs, value.rs (line-length wrapping) - black: remove extra blank line in test_v18.py - clippy: add #[allow(dead_code)] on ValueEndian::Little to silence the "variant never constructed" warning while keeping the variant for completeness Signed-off-by: Elad Venezian --- crates/qpy/src/circuit_reader.rs | 47 +++++++++++++++++++++----------- crates/qpy/src/circuit_writer.rs | 27 ++++++++++++------ crates/qpy/src/params.rs | 2 +- crates/qpy/src/py_methods.rs | 2 +- crates/qpy/src/value.rs | 20 +++++++++----- test/python/qpy/test_v18.py | 1 - 6 files changed, 65 insertions(+), 34 deletions(-) diff --git a/crates/qpy/src/circuit_reader.rs b/crates/qpy/src/circuit_reader.rs index 3cb3eb14840e..d2eb8c80b804 100644 --- a/crates/qpy/src/circuit_reader.rs +++ b/crates/qpy/src/circuit_reader.rs @@ -407,7 +407,8 @@ fn unpack_standard_gate( instruction.gate_class_name ))); }; - let param_values = get_instruction_values(instruction, qpy_data, ValueEndian::LittleForV17AndBelow)?; + let param_values = + get_instruction_values(instruction, qpy_data, ValueEndian::LittleForV17AndBelow)?; Ok((op, param_values)) } @@ -424,7 +425,8 @@ fn unpack_standard_instruction( instruction.gate_class_name ))); }; - let param_values = get_instruction_values(instruction, qpy_data, ValueEndian::LittleForV17AndBelow)?; + let param_values = + get_instruction_values(instruction, qpy_data, ValueEndian::LittleForV17AndBelow)?; Ok((op, param_values)) } @@ -500,7 +502,11 @@ fn unpack_pauli_product_rotation( "Pauli product rotation x parameter should be a boolean vector".to_string(), ) })?; - let angle_value = unpack_generic_value(&instruction.params[2], qpy_data, ValueEndian::LittleForV17AndBelow)?; + let angle_value = unpack_generic_value( + &instruction.params[2], + qpy_data, + ValueEndian::LittleForV17AndBelow, + )?; let angle = generic_value_to_param(&angle_value)?; let rotation = PauliProductRotation { z, x, angle }; let pbc = Box::new(PauliBased::PauliProductRotation(rotation)); @@ -513,8 +519,11 @@ fn unpack_unitary( instruction: &formats::CircuitInstructionV2Pack, qpy_data: &mut QPYReadData, ) -> Result<(PackedOperation, Vec), QpyError> { - let GenericValue::NumpyObject(bytes) = - unpack_generic_value(&instruction.params[0], qpy_data, ValueEndian::LittleForV17AndBelow)? + let GenericValue::NumpyObject(bytes) = unpack_generic_value( + &instruction.params[0], + qpy_data, + ValueEndian::LittleForV17AndBelow, + )? else { return Err(QpyError::InvalidParameter( "No matrix for unitary op".to_string(), @@ -573,7 +582,9 @@ fn unpack_control_flow( .params .iter() .skip(1) - .map(|param| unpack_generic_value(param, qpy_data, ValueEndian::LittleForV17AndBelow)) + .map(|param| { + unpack_generic_value(param, qpy_data, ValueEndian::LittleForV17AndBelow) + }) .collect::>()?; let duration_value = if let Some(duration_pack) = instruction.params.first() { unpack_duration_value(duration_pack, qpy_data)? @@ -737,15 +748,17 @@ fn unpack_control_flow( }; let label_spec_element = label_spec_element_tuple .iter() - .map(|label_spec_element| match label_spec_element.as_little_for_v17_and_below(qpy_data.version) { - GenericValue::CaseDefault => Ok(CaseSpecifier::Default), - GenericValue::BigInt(value) => Ok(CaseSpecifier::Uint(value.clone())), - GenericValue::Int64(value) => { - Ok(CaseSpecifier::Uint(BigUint::from(value as u64))) + .map(|label_spec_element| { + match label_spec_element.as_little_for_v17_and_below(qpy_data.version) { + GenericValue::CaseDefault => Ok(CaseSpecifier::Default), + GenericValue::BigInt(value) => Ok(CaseSpecifier::Uint(value.clone())), + GenericValue::Int64(value) => { + Ok(CaseSpecifier::Uint(BigUint::from(value as u64))) + } + _ => Err(QpyError::InvalidInstruction( + "could not identify switch case label spec".to_string(), + )), } - _ => Err(QpyError::InvalidInstruction( - "could not identify switch case label spec".to_string(), - )), }) .collect::>()?; label_spec.push(label_spec_element); @@ -775,7 +788,8 @@ fn unpack_py_instruction( qpy_data: &mut QPYReadData, ) -> Result<(PackedOperation, Vec), QpyError> { let name = instruction.gate_class_name.clone(); - let mut instruction_values = get_instruction_values(instruction, qpy_data, ValueEndian::LittleForV17AndBelow)?; + let mut instruction_values = + get_instruction_values(instruction, qpy_data, ValueEndian::LittleForV17AndBelow)?; Python::attach(|py| -> Result<_, QpyError> { let mut py_params: Vec> = instruction_values .iter() @@ -939,7 +953,8 @@ fn unpack_custom_instruction( let custom_instruction = custom_instructions_map.get(&name).ok_or_else(|| { QpyError::MissingData("Custom instruction data not found for {name}".to_string()) })?; - let instruction_values = get_instruction_values(instruction, qpy_data, ValueEndian::LittleForV17AndBelow)?; + let instruction_values = + get_instruction_values(instruction, qpy_data, ValueEndian::LittleForV17AndBelow)?; Python::attach(|py| -> Result<_, QpyError> { let py_params: Vec> = instruction_values .iter() diff --git a/crates/qpy/src/circuit_writer.rs b/crates/qpy/src/circuit_writer.rs index 85bc76be0769..987936cee918 100644 --- a/crates/qpy/src/circuit_writer.rs +++ b/crates/qpy/src/circuit_writer.rs @@ -55,7 +55,8 @@ use crate::py_methods::{ 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, + pack_generic_value, pack_standalone_var, pack_stretch, serialize, + serialize_param_register_value, }; use qiskit_circuit::var_stretch_container::{StretchType, VarType}; @@ -520,13 +521,13 @@ fn pack_control_flow_inst( match label_element { CaseSpecifier::Default => Ok(GenericValue::CaseDefault), CaseSpecifier::Uint(val) => { - let v = GenericValue::Int64( - val.to_i64().ok_or_else(|| { + let v = GenericValue::Int64(val.to_i64().ok_or_else( + || { QpyError::ConversionError( "Case specifier too large".to_string(), ) - })?, - ); + }, + )?); Ok(v.as_little_for_v17_and_below(qpy_data.version)) } } @@ -581,7 +582,11 @@ fn pack_unitary_gate( // we translate the matrix to numpy and then serialize it like python does let params = Python::attach(|py| -> Result<_, QpyError> { let out_array = matrix.to_pyarray(py); - Ok(vec![py_pack_param(&out_array, qpy_data, ValueEndian::LittleForV17AndBelow)?]) + Ok(vec![py_pack_param( + &out_array, + qpy_data, + ValueEndian::LittleForV17AndBelow, + )?]) })?; // since we won't recreate this gate via python, it's not important to verify the python name is identical to the one we use here // so we simply hard-code it instead of going through python @@ -612,12 +617,18 @@ fn pack_py_instruction( let py_op_object = py_inst.ob.bind(py); if py_op_object.is_instance(imports::CLIFFORD.get_bound(py))? { let tableau = py_op_object.getattr("tableau")?; - Ok(vec![py_pack_param(&tableau, qpy_data, ValueEndian::LittleForV17AndBelow)?]) + Ok(vec![py_pack_param( + &tableau, + qpy_data, + ValueEndian::LittleForV17AndBelow, + )?]) } else if py_op_object.is_instance(imports::ANNOTATED_OPERATION.get_bound(py))? { let modifiers = py_op_object.getattr("modifiers")?; modifiers .try_iter()? - .map(|modifier| py_pack_param(&modifier?, qpy_data, ValueEndian::LittleForV17AndBelow)) + .map(|modifier| { + py_pack_param(&modifier?, qpy_data, ValueEndian::LittleForV17AndBelow) + }) .collect::>() } else { pack_instruction_params(instruction, qpy_data) diff --git a/crates/qpy/src/params.rs b/crates/qpy/src/params.rs index f773a5f85e89..f3f75798a692 100644 --- a/crates/qpy/src/params.rs +++ b/crates/qpy/src/params.rs @@ -9,8 +9,8 @@ // Any modifications or derivative works of this code must retain this // copyright notice, and modified files need to carry a notice indicating // that they have been altered from the originals. -use binrw::Endian; use crate::value::ValueEndian; +use binrw::Endian; use num_complex::Complex64; use pyo3::prelude::*; use qiskit_circuit::operations::Param; diff --git a/crates/qpy/src/py_methods.rs b/crates/qpy/src/py_methods.rs index e76bb4b4e88a..84797ba3f32c 100644 --- a/crates/qpy/src/py_methods.rs +++ b/crates/qpy/src/py_methods.rs @@ -11,8 +11,8 @@ // that they have been altered from the originals. // Methods for QPY serialization working directly with Python-based data -use binrw::Endian; use crate::value::ValueEndian; +use binrw::Endian; use numpy::Complex64; use pyo3::IntoPyObjectExt; use pyo3::exceptions::PyTypeError; diff --git a/crates/qpy/src/value.rs b/crates/qpy/src/value.rs index 9f930e20dbc5..25abf91dec16 100644 --- a/crates/qpy/src/value.rs +++ b/crates/qpy/src/value.rs @@ -34,8 +34,8 @@ pub(crate) enum ValueEndian { Big, /// Always little-endian, regardless of QPY version. - /// Not currently used in QPY, but included for completeness and in case - /// a future format element genuinely requires little-endian encoding. + /// Not currently used, but included for completeness. + #[allow(dead_code)] Little, /// Little-endian for QPY v≤17, big-endian for QPY v≥18. @@ -58,7 +58,11 @@ impl ValueEndian { ValueEndian::Big => Endian::Big, ValueEndian::Little => Endian::Little, ValueEndian::LittleForV17AndBelow => { - if version >= 18 { Endian::Big } else { Endian::Little } + if version >= 18 { + Endian::Big + } else { + Endian::Little + } } } } @@ -426,7 +430,11 @@ impl GenericValue { /// Mirrors the `ValueEndian::LittleForV17AndBelow` variant — use this at the few remaining /// call sites that work directly with `GenericValue` rather than going through `load_value`. pub(crate) fn as_little_for_v17_and_below(&self, version: u8) -> Self { - if version < 18 { self.as_le() } else { self.clone() } + if version < 18 { + self.as_le() + } else { + self.clone() + } } pub(crate) fn as_circuit_data(&self) -> Option { match self { @@ -773,9 +781,7 @@ pub(crate) fn pack_for_collection(value: &ForCollection, version: u8) -> Generic match value { ForCollection::List(vec) => GenericValue::Tuple( vec.iter() - .map(|&val| { - GenericValue::Int64(val as i64).as_little_for_v17_and_below(version) - }) + .map(|&val| GenericValue::Int64(val as i64).as_little_for_v17_and_below(version)) .collect(), ), ForCollection::PyRange(py_range) => GenericValue::Range(*py_range), diff --git a/test/python/qpy/test_v18.py b/test/python/qpy/test_v18.py index 5a7467835970..6ffb2cf33f16 100644 --- a/test/python/qpy/test_v18.py +++ b/test/python/qpy/test_v18.py @@ -28,7 +28,6 @@ def _dump(qc: QuantumCircuit, version: int) -> bytes: return buf.getvalue() - class TestV17VsV18(QiskitTestCase): """Verify the binary-level differences between QPY v17 and v18.""" From fbef55facf1f97f6072545a9b0979e94ab04a178 Mon Sep 17 00:00:00 2001 From: Elad Venezian Date: Wed, 5 Aug 2026 16:00:31 +0300 Subject: [PATCH 05/10] Fix copyright year in test_v18.py (2026 -> 2025) Signed-off-by: Elad Venezian --- test/python/qpy/test_v18.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/python/qpy/test_v18.py b/test/python/qpy/test_v18.py index 6ffb2cf33f16..e70181a7df8f 100644 --- a/test/python/qpy/test_v18.py +++ b/test/python/qpy/test_v18.py @@ -1,6 +1,6 @@ # This code is part of Qiskit. # -# (C) Copyright IBM 2026. +# (C) Copyright IBM 2025. # # This code is licensed under the Apache License, Version 2.0. You may # obtain a copy of this license in the LICENSE.txt file in the root directory From b5a407f2a3afd214f34d64e872e89cef4213375c Mon Sep 17 00:00:00 2001 From: Elad Venezian Date: Wed, 5 Aug 2026 21:54:24 +0300 Subject: [PATCH 06/10] Fix QPY v18 round-trip failures and lint errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The CI round-trip tests for QPY v18 were failing because the test framework was generating Python writer/reader combinations for v17+, which are intentionally unsupported — Rust is the only supported codec for those versions. Root cause: all_qpy_combinations() in test_roundtrip.py generated (v18, Python, Rust) and (v18, Rust, Python) test cases by patching QPY_RUST_WRITE_MIN_VERSION. The Python codec was never updated to handle the v18 big-endian encoding, so round-trips silently produced wrong values (e.g. integer 1 read back as 72057594037927936). Fix: - Filter out Python writer/reader combinations for versions >= QPY_RUST_WRITE_MIN_VERSION in all_qpy_combinations(), keeping Python only for v13-16 where it is the only option. - Add explicit QpyError assertions in write_circuit() and read_circuit() in binary_io/circuits.py: if the Python path is entered for a version that requires Rust, raise immediately with a clear message rather than silently producing corrupt output. This makes the version boundary self-documenting and prevents future regressions. Also fix test/python/qpy/test_v18.py license header (http -> https). Signed-off-by: Elad Venezian --- qiskit/qpy/binary_io/circuits.py | 10 ++++++++++ test/python/qpy/test_roundtrip.py | 3 +++ test/python/qpy/test_v18.py | 2 +- 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/qiskit/qpy/binary_io/circuits.py b/qiskit/qpy/binary_io/circuits.py index 938b53011c50..c4e62b3b8022 100644 --- a/qiskit/qpy/binary_io/circuits.py +++ b/qiskit/qpy/binary_io/circuits.py @@ -1549,6 +1549,11 @@ def write_circuit( annotation_factories=annotation_factories, ) return + if version >= common.QPY_RUST_WRITE_MIN_VERSION: + raise QpyError( + f"QPY version {version} is not supported by the Python writer. " + f"The Python writer only supports versions below {common.QPY_RUST_WRITE_MIN_VERSION}." + ) annotation_state = _AnnotationSerializationState(annotation_factories or {}) metadata_raw = json.dumps( circuit.metadata, separators=(",", ":"), cls=metadata_serializer @@ -1696,6 +1701,11 @@ def read_circuit( return _qpy.read_circuit( file_obj, version, metadata_deserializer, use_symengine, annotation_factories ) + if version >= common.QPY_RUST_READ_MIN_VERSION: + raise QpyError( + f"QPY version {version} is not supported by the Python reader. " + f"The Python reader only supports versions below {common.QPY_RUST_READ_MIN_VERSION}." + ) vectors = {} if version < 2: diff --git a/test/python/qpy/test_roundtrip.py b/test/python/qpy/test_roundtrip.py index 2e459621d394..0afe76140657 100644 --- a/test/python/qpy/test_roundtrip.py +++ b/test/python/qpy/test_roundtrip.py @@ -44,6 +44,9 @@ def wrapper(func): for read_with in ( ("Python", "Rust") if version >= QPY_RUST_READ_MIN_VERSION else ("Python",) ) + # Python writer/reader are not supported for v >= QPY_RUST_WRITE_MIN_VERSION + if not (version >= QPY_RUST_WRITE_MIN_VERSION and write_with == "Python") + if not (version >= QPY_RUST_WRITE_MIN_VERSION and read_with == "Python") )(unpack(func)) return wrapper diff --git a/test/python/qpy/test_v18.py b/test/python/qpy/test_v18.py index e70181a7df8f..413d74a29a76 100644 --- a/test/python/qpy/test_v18.py +++ b/test/python/qpy/test_v18.py @@ -4,7 +4,7 @@ # # This code is licensed under the Apache License, Version 2.0. You may # obtain a copy of this license in the LICENSE.txt file in the root directory -# of this source tree or at http://www.apache.org/licenses/LICENSE-2.0. +# of this source tree or at https://www.apache.org/licenses/LICENSE-2.0. # # Any modifications or derivative works of this code must retain this # copyright notice, and modified files need to carry a notice indicating From 83b3dbfc6507993fba0945ecd79b3109ac51d24b Mon Sep 17 00:00:00 2001 From: Elad Date: Thu, 6 Aug 2026 14:01:20 +0300 Subject: [PATCH 07/10] Apply suggestion from @ShellyGarion Co-authored-by: Shelly Garion <46566946+ShellyGarion@users.noreply.github.com> --- test/python/qpy/test_v18.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/python/qpy/test_v18.py b/test/python/qpy/test_v18.py index 413d74a29a76..724ec27f6e0a 100644 --- a/test/python/qpy/test_v18.py +++ b/test/python/qpy/test_v18.py @@ -1,6 +1,6 @@ # This code is part of Qiskit. # -# (C) Copyright IBM 2025. +# (C) Copyright IBM 2026. # # This code is licensed under the Apache License, Version 2.0. You may # obtain a copy of this license in the LICENSE.txt file in the root directory From 7dae5fe6ea61ccd28728868a5ba0384ff2e7f215 Mon Sep 17 00:00:00 2001 From: mohamedmahameed Date: Wed, 12 Aug 2026 17:27:37 +0300 Subject: [PATCH 08/10] replace the x00-prefixed string encoding of register payloads with a tag Signed-off-by: mohamedmahameed --- crates/qpy/src/formats.rs | 27 +++++- crates/qpy/src/value.rs | 96 +++++++++++++------ qiskit/qpy/__init__.py | 46 +++++++-- qiskit/qpy/binary_io/circuits.py | 67 +++++++++++-- qiskit/qpy/formats.py | 16 ++++ ...y-18-tagged-register-6b1f0a3c5d92e47a.yaml | 8 ++ test/python/qpy/test_roundtrip.py | 65 +++++++++++++ 7 files changed, 277 insertions(+), 48 deletions(-) create mode 100644 releasenotes/notes/qpy-18-tagged-register-6b1f0a3c5d92e47a.yaml diff --git a/crates/qpy/src/formats.rs b/crates/qpy/src/formats.rs index c750bc8d565e..93c811998a2f 100644 --- a/crates/qpy/src/formats.rs +++ b/crates/qpy/src/formats.rs @@ -254,7 +254,8 @@ pub struct RegisterV4Pack { // 1) None. // 2) Two-tuple: a tuple of the form (register, target) where the register value should be compared with the target. // In this case the target is a python int, represented in rust as BigUInt, but in python qpy it was saved using i64 so we keep it for now. -// Note that we also use (clbit, bool_target) as two tuple, where the clbit is encoded using the "\x00" hack that can be seen in ParamRegisterValue +// Note that we also use (clbit, bool_target) as two tuple; register and clbit alike are encoded as +// a register payload, see `ParamRegisterPack` below // 3) Expression // In the two-tuple representation, the target value is stored in the `value` field and the number of bytes in the serialized registered are stored in the // `register_size` fields. Both are unused in the other cases, making the packing and decoding of this struct rather non-uniform. @@ -286,8 +287,9 @@ impl TryFrom for ConditionType { } } -// register SHOULD be a string, but since we encode some registers starting with "\x00" they are rendered illegal -// we should probably change this in future versions to support magic numbers (TODO: change in QPY18?) +// The condition's register is carried as an opaque blob because its encoding depends on the QPY +// version: up to 17 it is the string hack described on `ParamRegisterPack`, from 18 it is that +// tagged struct. `value::load_param_register_value` decodes it. #[derive(Debug)] pub enum ConditionData { None, @@ -295,6 +297,25 @@ pub enum ConditionData { Expression(GenericDataPack), } +/// A `Register` payload, which is either a whole `ClassicalRegister` or a single `Clbit`. It +#[binrw] +#[brw(big)] +#[derive(Debug)] +pub enum ParamRegisterPack { + /// A classical register, identified by name. The name runs to the end of the payload, whose + /// length the enclosing field already carries (`condition_register_size` for a condition, the + /// `INSTRUCTION_PARAM` header's `size` for a parameter), so it needs no length of its own. + #[brw(magic = 1u8)] + Register { + #[br(parse_with = binrw::helpers::until_eof, try_map = String::from_utf8)] + #[bw(map = |name| name.as_bytes())] + name: String, + }, + /// A single clbit, identified by its index in the circuit's clbit list. + #[brw(magic = 0u8)] + Clbit { index: u32 }, +} + // most of the data here is "virtual" in the sense that is is not stored as-is // we use custom reader/writer to enable spreading the data to its relevant places in the qpy instruction // in newer versions of qpy it may be better to store all the data consecutively diff --git a/crates/qpy/src/value.rs b/crates/qpy/src/value.rs index 25abf91dec16..eac163b4fd03 100644 --- a/crates/qpy/src/value.rs +++ b/crates/qpy/src/value.rs @@ -991,10 +991,10 @@ pub(crate) fn unpack_duration(duration_pack: DurationPack) -> Duration { } // due to historical reasons, the treatment of instructions params which are registers/clbits is a little strange -// When a register is stored as an instruction param, it is serialized compactly -// For a classical register its name is saved as a string; for a clbit -// its index in the full clbit list is converted into a string, with 0x00 appended at the start -// to differentiate from the register case +// A `Register` value stored inside an instruction is either a whole classical register or a single +// clbit. From QPY 18 the two are told apart by a tag byte (`formats::ParamRegisterPack`); up to +// QPY 17 they shared one untyped string, a register being its bare name and a clbit being 0x00 +// followed by its index in ASCII digits. #[derive(Debug, PartialEq, Clone)] pub enum ParamRegisterValue { @@ -1006,17 +1006,22 @@ pub(crate) fn serialize_param_register_value( value: &ParamRegisterValue, qpy_data: &QPYWriteData, ) -> Result { + if qpy_data.version >= 18 { + let pack = match value { + ParamRegisterValue::Register(register) => formats::ParamRegisterPack::Register { + name: register.name().to_string(), + }, + ParamRegisterValue::ShareableClbit(clbit) => formats::ParamRegisterPack::Clbit { + index: find_clbit_index(clbit, qpy_data)?.0, + }, + }; + return serialize(&pack); + } + // QPY <= 17: the untyped string form. match value { ParamRegisterValue::Register(register) => Ok(register.name().into()), ParamRegisterValue::ShareableClbit(clbit) => { - let name = qpy_data - .circuit_data - .clbits() - .find(clbit) - .ok_or_else(|| QpyError::InvalidBit("clbit not found".to_string()))? - .0 - .to_string(); - // this is the part where we get hack-y + let name = find_clbit_index(clbit, qpy_data)?.0.to_string(); let mut bytes: Bytes = Bytes(Vec::with_capacity(name.len() + 1)); bytes.push(0u8); bytes.extend_from_slice(name.as_bytes()); @@ -1029,7 +1034,18 @@ pub(crate) fn load_param_register_value( bytes: &Bytes, qpy_data: &mut QPYReadData, ) -> Result { - // If register name prefixed with null character it's a clbit index for single bit condition. + if qpy_data.version >= 18 { + let (pack, _) = deserialize::(bytes)?; + return match pack { + formats::ParamRegisterPack::Register { name } => { + Ok(ParamRegisterValue::Register(find_creg(&name, qpy_data)?)) + } + formats::ParamRegisterPack::Clbit { index } => Ok(ParamRegisterValue::ShareableClbit( + find_clbit(Clbit(index), qpy_data)?, + )), + }; + } + // QPY <= 17: a leading null character means the rest is a clbit index in ASCII digits. if bytes.is_empty() { return Err(QpyError::InvalidRegister( "Failed to load register - name missing".to_string(), @@ -1041,26 +1057,44 @@ pub(crate) fn load_param_register_value( QpyError::ConversionError(format!("Failed to parse clbit index: {}", e)) }, )?); - match qpy_data.circuit_data.clbits().get(index) { - Some(shareable_clbit) => { - Ok(ParamRegisterValue::ShareableClbit(shareable_clbit.clone())) - } - None => Err(QpyError::InvalidBit(format!( - "Could not find clbit {:?}", - index - ))), - } + Ok(ParamRegisterValue::ShareableClbit(find_clbit( + index, qpy_data, + )?)) } else { // `bytes` has the register name let name = std::str::from_utf8(bytes)?; - for creg in qpy_data.circuit_data.cregs() { - if creg.name() == name { - return Ok(ParamRegisterValue::Register(creg.clone())); - } - } - Err(QpyError::InvalidRegister(format!( - "Could not find classical register {:?}", - name - ))) + Ok(ParamRegisterValue::Register(find_creg(name, qpy_data)?)) } } + +/// Position of `clbit` in the circuit being written, which is how both encodings identify it. +fn find_clbit_index(clbit: &ShareableClbit, qpy_data: &QPYWriteData) -> Result { + qpy_data + .circuit_data + .clbits() + .find(clbit) + .ok_or_else(|| QpyError::InvalidBit("clbit not found".to_string())) +} + +/// The clbit at `index` in the circuit being read. +fn find_clbit(index: Clbit, qpy_data: &QPYReadData) -> Result { + qpy_data + .circuit_data + .clbits() + .get(index) + .cloned() + .ok_or_else(|| QpyError::InvalidBit(format!("Could not find clbit {:?}", index))) +} + +/// The classical register called `name` in the circuit being read. +fn find_creg(name: &str, qpy_data: &QPYReadData) -> Result { + qpy_data + .circuit_data + .cregs() + .iter() + .find(|creg| creg.name() == name) + .cloned() + .ok_or_else(|| { + QpyError::InvalidRegister(format!("Could not find classical register {:?}", name)) + }) +} diff --git a/qiskit/qpy/__init__.py b/qiskit/qpy/__init__.py index 96d5bee2cf0b..5aa7da64f42a 100644 --- a/qiskit/qpy/__init__.py +++ b/qiskit/qpy/__init__.py @@ -476,6 +476,33 @@ def open(*args): to big-endian byte order, consistent with the rest of the QPY specification. In versions 1–17 these were mistakenly written in little-endian. +New ParamRegisterPack +~~~~~~~~~~~~~~~~~~~~~ +Version 18 replaces the encoding of a `Register` payload, which stores either a whole +:class:`.ClassicalRegister` or a single :class:`.Clbit`. It appears as an instruction's condition +(see :ref:`qpy_instructions`) and as an ``INSTRUCTION_PARAM`` of type ``'R'``. + +Up to :ref:`version 17 ` both cases shared one untyped utf8 string: a register was +its bare name, while a clbit was a null character ``"\\x00"`` followed by the bit's index in the +circuit *written out as decimal digits*. From version 18 the payload begins with a tag byte +identifying which of the two it is: + +.. code-block:: c + + struct { // classical register, tag == 1 + uint8_t kind; + char name[]; // to the end of the payload + } + + struct { // single clbit, tag == 0 + uint8_t kind; + uint32_t index; // index of the bit in the circuit + } + +The register name needs no length of its own because the enclosing field already delimits the +payload: ``conditional_reg_name_size`` for a condition, and the ``INSTRUCTION_PARAM`` header's +``size`` for a parameter. + .. _qpy_version_17: Version 17 @@ -2117,11 +2144,14 @@ class if it's defined in Qiskit. Otherwise it falls back to the custom instruction name. Following the ``name`` bytes there are ``label_size`` bytes of utf8 data for the label if one was set on the instruction. Following the label bytes if ``has_conditional`` is ``True`` then there are -``conditional_reg_name_size`` bytes of utf8 data for the name of the conditional -register name. In case of single classical bit conditions the register name -utf8 data will be prefixed with a null character "\\x00" and then a utf8 string -integer representing the classical bit index in the circuit that the condition -is on. +``conditional_reg_name_size`` bytes holding the condition's `Register` payload, which +identifies either a classical register or a single classical bit. + +Up to :ref:`version 17 ` that payload is utf8 data giving the name of the +conditional register, and in case of single classical bit conditions the register name utf8 data +will be prefixed with a null character "\\x00" and then a utf8 string integer representing the +classical bit index in the circuit that the condition is on. From +:ref:`version 18 ` onwards it is the tagged struct described in that section. This is immediately followed by the INSTRUCTION_ARG structs for the list of arguments of that instruction. These are in the order of all quantum arguments @@ -2152,8 +2182,10 @@ class if it's defined in Qiskit. Otherwise it falls back to the custom } After each INSTRUCTION_PARAM the next ``size`` bytes are the parameter's data. -The ``type`` field can be ``'i'``, ``'f'``, ``'p'``, ``'e'``, ``'s'``, ``'c'`` -or ``'n'`` which dictate the format. For ``'i'`` it's an integer, ``'f'`` it's +The ``type`` field can be ``'i'``, ``'f'``, ``'p'``, ``'e'``, ``'s'``, ``'c'``, +``'R'`` or ``'n'`` which dictate the format. ``'R'`` is a `REGISTER_PARAM` payload, +identifying a :class:`.ClassicalRegister` or a single :class:`.Clbit` in the encoding described in +:ref:`version 18 `. For ``'i'`` it's an integer, ``'f'`` it's a double, ``'s'`` if it's a string (encoded as utf8), ``'c'`` is a complex and the data is represented by the struct format in the :ref:`qpy_param_expr` section. ``'p'`` defines a :class:`~qiskit.circuit.Parameter` object which is diff --git a/qiskit/qpy/binary_io/circuits.py b/qiskit/qpy/binary_io/circuits.py index c4e62b3b8022..725a2c28449e 100644 --- a/qiskit/qpy/binary_io/circuits.py +++ b/qiskit/qpy/binary_io/circuits.py @@ -332,7 +332,7 @@ def _loads_instruction_parameter( # TODO This uses little endian. Should be fixed in the next QPY version. param = struct.unpack("= 18: + if not data_bytes: + raise QpyError("Malformed REGISTER_PARAM payload: no tag byte") + (tag,) = struct.unpack_from(formats.REGISTER_PARAM_TAG_PACK, data_bytes) + if tag == formats.REGISTER_PARAM_TAG_CLBIT: + if len(data_bytes) != formats.REGISTER_PARAM_CLBIT_SIZE: + raise QpyError( + f"Malformed REGISTER_PARAM payload: a clbit occupies " + f"{formats.REGISTER_PARAM_CLBIT_SIZE} bytes, got {len(data_bytes)}" + ) + clbit = formats.REGISTER_PARAM_CLBIT._make( + struct.unpack(formats.REGISTER_PARAM_CLBIT_PACK, data_bytes) + ) + if clbit.index >= len(circuit.clbits): + raise QpyError( + f"Malformed REGISTER_PARAM payload: clbit index {clbit.index} is out of range for a " + f"circuit with {len(circuit.clbits)} clbit(s)" + ) + return circuit.clbits[clbit.index] + if tag != formats.REGISTER_PARAM_TAG_REGISTER: + raise QpyError(f"Malformed REGISTER_PARAM payload: unknown tag {tag}") + name = data_bytes[formats.REGISTER_PARAM_TAG_SIZE :].decode(common.ENCODE) + if name not in registers["c"]: + raise QpyError( + f"Malformed REGISTER_PARAM payload: no classical register named {name!r}" + ) + return registers["c"][name] + data_bytes = data_bytes.decode(common.ENCODE) # If register name prefixed with null character it's a clbit index for single bit condition. if data_bytes[0] == "\x00": conditional_bit = int(data_bytes[1:]) @@ -393,14 +430,14 @@ def _read_instruction( gate_name = file_obj.read(instruction.name_size).decode(common.ENCODE) label = file_obj.read(instruction.label_size).decode(common.ENCODE) - condition_register = file_obj.read(instruction.condition_register_size).decode(common.ENCODE) + condition_register = file_obj.read(instruction.condition_register_size) qargs = [] cargs = [] params = [] condition = None if conditional_key == type_keys.Condition.TWO_TUPLE: condition = ( - _loads_register_param(condition_register, circuit, registers), + _loads_register_param(condition_register, circuit, registers, version), instruction.condition_value, ) elif conditional_key == type_keys.Condition.EXPRESSION: @@ -891,7 +928,23 @@ def _read_calibrations(file_obj, version, vectors, metadata_deserializer): schedules.read_schedule_block(file_obj, version, metadata_deserializer) -def _py_serialize_register_param(register, index_map): +def _py_serialize_register_param(register, index_map, version): + """Serialize a REGISTER_PARAM payload: either a whole classical register or a single clbit. + + From QPY 18 a tag byte says which, followed by the register name (to the end of the payload) or + the clbit index. Up to QPY 17 both shared one untyped string; see :mod:`qiskit.qpy.formats`. + """ + if version >= 18: + if isinstance(register, ClassicalRegister): + return struct.pack( + formats.REGISTER_PARAM_TAG_PACK, formats.REGISTER_PARAM_TAG_REGISTER + ) + register.name.encode(common.ENCODE) + # Clbit. + return struct.pack( + formats.REGISTER_PARAM_CLBIT_PACK, + formats.REGISTER_PARAM_TAG_CLBIT, + index_map["c"][register], + ) if isinstance(register, ClassicalRegister): return register.name.encode(common.ENCODE) # Clbit. @@ -937,7 +990,7 @@ def _dumps_instruction_parameter( data_bytes = struct.pack("=18 +# +# A Register param payload holds either a whole classical register or a single clbit, told apart by a +# leading tag byte. A register's name follows the tag and runs to the end of the payload, whose +# length the enclosing field already gives; a clbit is stored as its index in the circuit. +# +# Up to version 17 both cases shared one untyped string: a register was its bare name, and a clbit +# was a b"\x00" byte followed by its index written out as ASCII digits. +REGISTER_PARAM_TAG_REGISTER = 1 +REGISTER_PARAM_TAG_CLBIT = 0 +REGISTER_PARAM_TAG_PACK = "!B" +REGISTER_PARAM_TAG_SIZE = struct.calcsize(REGISTER_PARAM_TAG_PACK) +REGISTER_PARAM_CLBIT = namedtuple("REGISTER_PARAM_CLBIT", ["tag", "index"]) +REGISTER_PARAM_CLBIT_PACK = "!BI" +REGISTER_PARAM_CLBIT_SIZE = struct.calcsize(REGISTER_PARAM_CLBIT_PACK) + # Pauli Evolution Gate PAULI_EVOLUTION_DEF = namedtuple( "PAULI_EVOLUTION_DEF", diff --git a/releasenotes/notes/qpy-18-tagged-register-6b1f0a3c5d92e47a.yaml b/releasenotes/notes/qpy-18-tagged-register-6b1f0a3c5d92e47a.yaml new file mode 100644 index 000000000000..55c6975b0a95 --- /dev/null +++ b/releasenotes/notes/qpy-18-tagged-register-6b1f0a3c5d92e47a.yaml @@ -0,0 +1,8 @@ +--- +upgrade_qpy: + - | + QPY has been upgraded to version 18, which changes how a ``REGISTER_PARAM`` payload -- the + field that identifies either a :class:`.ClassicalRegister` or a single :class:`.Clbit` -- tells + the two apart. It now begins with a tag byte and stores a clbit as a ``uint32_t`` index, + instead of overloading one string in which a clbit was a null character followed by its index + in decimal digits. See :ref:`qpy_version_18` for the format description. diff --git a/test/python/qpy/test_roundtrip.py b/test/python/qpy/test_roundtrip.py index 0afe76140657..e946fc34bf1f 100644 --- a/test/python/qpy/test_roundtrip.py +++ b/test/python/qpy/test_roundtrip.py @@ -107,6 +107,71 @@ def test_ifelse(self, version, write_with, read_with): qc.if_else(condition, body, false_body, [qc.qubits[0]], []) self.assert_roundtrip_equal(qc, version=version, read_with=read_with, write_with=write_with) + @all_qpy_combinations(QPY_RUST_READ_MIN_VERSION) + def test_ifelse_single_clbit_condition(self, version, write_with, read_with): + """Check an IfElse conditioned on a single clbit rather than a whole register. + + The two share one ``REGISTER`` payload, whose encoding changed in QPY 18, so both arms need + covering: ``test_ifelse`` conditions on a register and this one on a bit. + """ + qc = QuantumCircuit(2, 2) + body = QuantumCircuit([qc.qubits[0]]) + body.x(0) + qc.if_test((qc.clbits[1], True), body, [qc.qubits[0]], []) + self.assert_roundtrip_equal(qc, version=version, read_with=read_with, write_with=write_with) + + @all_qpy_combinations(QPY_RUST_READ_MIN_VERSION) + def test_ifelse_single_clbit_condition_nonzero_index(self, version, write_with, read_with): + qc = QuantumCircuit(3, 4) + body = QuantumCircuit([qc.qubits[0]]) + print("---------------------") + print(qc.qubits[0]) + print("---------------------") + body.x(0) + + qc.if_test((qc.clbits[2], True), body, [qc.qubits[0]], []) + + self.assert_roundtrip_equal( + qc, + version=version, + read_with=read_with, + write_with=write_with, + ) + + @all_qpy_combinations(QPY_RUST_READ_MIN_VERSION) + def test_ifelse_single_clbit_condition_index_zero(self, version, write_with, read_with): + """Check a bit condition on clbit 0, whose index is the edge case of both encodings. + + Up to QPY 17 it is the shortest possible payload (a null byte and the digit ``0``), and from + QPY 18 it is the only one whose ``uint32_t`` index is entirely zero bytes. + """ + qc = QuantumCircuit(2, 2) + body = QuantumCircuit([qc.qubits[0]]) + body.x(0) + qc.if_test((qc.clbits[0], True), body, [qc.qubits[0]], []) + self.assert_roundtrip_equal(qc, version=version, read_with=read_with, write_with=write_with) + + @all_qpy_combinations(QPY_RUST_READ_MIN_VERSION) + def test_ifelse_condition_among_multiple_registers(self, version, write_with, read_with): + """Check that a condition resolves to the right register when several are present. + + The payload identifies a register by name and a bit by its circuit-wide index, so a circuit + with more than one same-sized register exercises the lookup rather than just the encoding. + """ + qr = QuantumRegister(1, "q") + cregs = [ClassicalRegister(2, name) for name in ("alpha", "beta", "gamma")] + body = QuantumCircuit(1) + body.x(0) + + qc = QuantumCircuit(qr, *cregs) + qc.if_test((cregs[1], 1), body.copy(), [qr[0]], []) + self.assert_roundtrip_equal(qc, version=version, read_with=read_with, write_with=write_with) + + # `gamma[1]` is clbit 5 of the circuit; the payload stores that circuit-wide index. + qc = QuantumCircuit(qr, *cregs) + qc.if_test((cregs[2][1], True), body.copy(), [qr[0]], []) + self.assert_roundtrip_equal(qc, version=version, read_with=read_with, write_with=write_with) + @all_qpy_combinations(QPY_RUST_READ_MIN_VERSION) def test_box(self, version, write_with, read_with): """Check the BoxOp control flow gate passes roundtrip""" From 5faa34b962fafd87f5f25c9a648cf27bc825688f Mon Sep 17 00:00:00 2001 From: mohamedmahameed Date: Tue, 18 Aug 2026 12:19:57 +0300 Subject: [PATCH 09/10] prints in code cleanup Signed-off-by: mohamedmahameed --- test/python/qpy/test_roundtrip.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/test/python/qpy/test_roundtrip.py b/test/python/qpy/test_roundtrip.py index e946fc34bf1f..5e40f9464f03 100644 --- a/test/python/qpy/test_roundtrip.py +++ b/test/python/qpy/test_roundtrip.py @@ -124,9 +124,6 @@ def test_ifelse_single_clbit_condition(self, version, write_with, read_with): def test_ifelse_single_clbit_condition_nonzero_index(self, version, write_with, read_with): qc = QuantumCircuit(3, 4) body = QuantumCircuit([qc.qubits[0]]) - print("---------------------") - print(qc.qubits[0]) - print("---------------------") body.x(0) qc.if_test((qc.clbits[2], True), body, [qc.qubits[0]], []) From bcd0a67c140c28a853855b7d580379b738e6660e Mon Sep 17 00:00:00 2001 From: mohamedmahameed Date: Tue, 18 Aug 2026 14:23:40 +0300 Subject: [PATCH 10/10] add tests to test_v18.py, code enhancements, documentation arrangements Signed-off-by: mohamedmahameed --- crates/qpy/src/expr.rs | 33 +++--------- crates/qpy/src/value.rs | 58 +++++++++++--------- qiskit/qpy/__init__.py | 21 ++++---- test/python/qpy/test_roundtrip.py | 62 --------------------- test/python/qpy/test_v18.py | 90 ++++++++++++++++++++++++++++++- 5 files changed, 139 insertions(+), 125 deletions(-) diff --git a/crates/qpy/src/expr.rs b/crates/qpy/src/expr.rs index 6bcc4f12556e..dcb67b5a4be4 100644 --- a/crates/qpy/src/expr.rs +++ b/crates/qpy/src/expr.rs @@ -17,11 +17,11 @@ use crate::formats::{ ExpressionVarElementPack, ExpressionVarRegisterPack, }; use crate::value::{ - QPYReadData, QPYWriteData, pack_biguint, pack_duration, unpack_biguint, unpack_duration, + QPYReadData, QPYWriteData, clbit_at, clbit_index, creg_by_name, pack_biguint, pack_duration, + unpack_biguint, unpack_duration, }; use binrw::{BinRead, BinResult, BinWrite, Endian, Error}; use num_bigint::BigUint; -use qiskit_circuit::Clbit; use qiskit_circuit::classical::expr::{ Binary, BinaryOp, Cast, Expr, Index, Unary, UnaryOp, Value, Var, }; @@ -109,16 +109,7 @@ pub(crate) fn pack_expression_var( let (ty, value_pack) = match var { Var::Bit { bit } => ( &Type::Bool, - ExpressionVarElementPack::Clbit( - qpy_data - .circuit_data - .clbits() - .find(bit) - .ok_or_else(|| { - QpyError::InvalidBit(format!("Could not find bit {:?} in circuit", bit)) - })? - .0, - ), + ExpressionVarElementPack::Clbit(clbit_index(bit, qpy_data)?), ), Var::Register { register, ty } => ( ty, @@ -152,24 +143,12 @@ pub(crate) fn unpack_expression_var( let ty = unpack_expression_type(var_type_pack); match var_element_pack { ExpressionVarElementPack::Clbit(index) => Ok(Var::Bit { - bit: qpy_data - .circuit_data - .clbits() - .get(Clbit(index)) - .ok_or_else(|| QpyError::InvalidBit("Clbit not found in circuit data".to_string()))? - .clone(), + bit: clbit_at(index, qpy_data)?, }), ExpressionVarElementPack::Register(packed_register) => Ok(Var::Register { - register: qpy_data - .circuit_data - .cregs_data() - .get(packed_register.name.as_str()) - .ok_or_else(|| { - QpyError::InvalidRegister("Register not found in circuit data".to_string()) - })? - .clone(), + register: creg_by_name(&packed_register.name, qpy_data)?, ty, - }), // TODO: can we avoid cloning? + }), ExpressionVarElementPack::Uuid(key) => { let var = qpy_data.standalone_vars.get(&key).ok_or_else(|| { QpyError::InvalidParameter("Standalone var not found in qpy data".to_string()) diff --git a/crates/qpy/src/value.rs b/crates/qpy/src/value.rs index eac163b4fd03..4f0eb5b66a08 100644 --- a/crates/qpy/src/value.rs +++ b/crates/qpy/src/value.rs @@ -1012,7 +1012,7 @@ pub(crate) fn serialize_param_register_value( name: register.name().to_string(), }, ParamRegisterValue::ShareableClbit(clbit) => formats::ParamRegisterPack::Clbit { - index: find_clbit_index(clbit, qpy_data)?.0, + index: clbit_index(clbit, qpy_data)?, }, }; return serialize(&pack); @@ -1021,7 +1021,7 @@ pub(crate) fn serialize_param_register_value( match value { ParamRegisterValue::Register(register) => Ok(register.name().into()), ParamRegisterValue::ShareableClbit(clbit) => { - let name = find_clbit_index(clbit, qpy_data)?.0.to_string(); + let name = clbit_index(clbit, qpy_data)?.to_string(); let mut bytes: Bytes = Bytes(Vec::with_capacity(name.len() + 1)); bytes.push(0u8); bytes.extend_from_slice(name.as_bytes()); @@ -1038,10 +1038,10 @@ pub(crate) fn load_param_register_value( let (pack, _) = deserialize::(bytes)?; return match pack { formats::ParamRegisterPack::Register { name } => { - Ok(ParamRegisterValue::Register(find_creg(&name, qpy_data)?)) + Ok(ParamRegisterValue::Register(creg_by_name(&name, qpy_data)?)) } formats::ParamRegisterPack::Clbit { index } => Ok(ParamRegisterValue::ShareableClbit( - find_clbit(Clbit(index), qpy_data)?, + clbit_at(index, qpy_data)?, )), }; } @@ -1052,49 +1052,59 @@ pub(crate) fn load_param_register_value( )); } if bytes[0] == 0u8 { - let index = Clbit(std::str::from_utf8(&bytes[1..])?.parse().map_err( - |e: std::num::ParseIntError| { - QpyError::ConversionError(format!("Failed to parse clbit index: {}", e)) - }, - )?); - Ok(ParamRegisterValue::ShareableClbit(find_clbit( + let index: u32 = + std::str::from_utf8(&bytes[1..])? + .parse() + .map_err(|e: std::num::ParseIntError| { + QpyError::ConversionError(format!("Failed to parse clbit index: {e}")) + })?; + Ok(ParamRegisterValue::ShareableClbit(clbit_at( index, qpy_data, )?)) } else { // `bytes` has the register name let name = std::str::from_utf8(bytes)?; - Ok(ParamRegisterValue::Register(find_creg(name, qpy_data)?)) + Ok(ParamRegisterValue::Register(creg_by_name(name, qpy_data)?)) } } -/// Position of `clbit` in the circuit being written, which is how both encodings identify it. -fn find_clbit_index(clbit: &ShareableClbit, qpy_data: &QPYWriteData) -> Result { +/// Position of `clbit` in the circuit being written, which is how the format refers to a bit. +/// +/// A thin wrapper over [`CircuitData::clbit_index`] that turns the `None` into the QPY error, so +/// every site that needs a bit index reports the same thing. +pub(crate) fn clbit_index( + clbit: &ShareableClbit, + qpy_data: &QPYWriteData, +) -> Result { qpy_data .circuit_data - .clbits() - .find(clbit) - .ok_or_else(|| QpyError::InvalidBit("clbit not found".to_string())) + .clbit_index(clbit) + .ok_or_else(|| QpyError::InvalidBit(format!("Could not find clbit {clbit:?} in circuit"))) } /// The clbit at `index` in the circuit being read. -fn find_clbit(index: Clbit, qpy_data: &QPYReadData) -> Result { +pub(crate) fn clbit_at(index: u32, qpy_data: &QPYReadData) -> Result { qpy_data .circuit_data .clbits() - .get(index) + .get(Clbit(index)) .cloned() - .ok_or_else(|| QpyError::InvalidBit(format!("Could not find clbit {:?}", index))) + .ok_or_else(|| QpyError::InvalidBit(format!("Could not find clbit {index} in circuit"))) } /// The classical register called `name` in the circuit being read. -fn find_creg(name: &str, qpy_data: &QPYReadData) -> Result { +/// +/// Uses the name-keyed [`CircuitData::cregs_data`] map rather than scanning `cregs()`. +pub(crate) fn creg_by_name( + name: &str, + qpy_data: &QPYReadData, +) -> Result { qpy_data .circuit_data - .cregs() - .iter() - .find(|creg| creg.name() == name) + .cregs_data() + .get(name) .cloned() .ok_or_else(|| { - QpyError::InvalidRegister(format!("Could not find classical register {:?}", name)) + QpyError::InvalidRegister(format!("Could not find classical register {name:?}")) }) } diff --git a/qiskit/qpy/__init__.py b/qiskit/qpy/__init__.py index 5aa7da64f42a..a6c6e920c428 100644 --- a/qiskit/qpy/__init__.py +++ b/qiskit/qpy/__init__.py @@ -2144,14 +2144,15 @@ class if it's defined in Qiskit. Otherwise it falls back to the custom instruction name. Following the ``name`` bytes there are ``label_size`` bytes of utf8 data for the label if one was set on the instruction. Following the label bytes if ``has_conditional`` is ``True`` then there are -``conditional_reg_name_size`` bytes holding the condition's `Register` payload, which -identifies either a classical register or a single classical bit. +``conditional_reg_name_size`` bytes of utf8 data for the name of the conditional +register name. In case of single classical bit conditions the register name +utf8 data will be prefixed with a null character "\\x00" and then a utf8 string +integer representing the classical bit index in the circuit that the condition +is on. -Up to :ref:`version 17 ` that payload is utf8 data giving the name of the -conditional register, and in case of single classical bit conditions the register name utf8 data -will be prefixed with a null character "\\x00" and then a utf8 string integer representing the -classical bit index in the circuit that the condition is on. From -:ref:`version 18 ` onwards it is the tagged struct described in that section. +.. versionchanged:: QPY 18 + This payload is a tagged struct rather than a utf8 string; see + :ref:`qpy_version_18`. This is immediately followed by the INSTRUCTION_ARG structs for the list of arguments of that instruction. These are in the order of all quantum arguments @@ -2182,10 +2183,8 @@ class if it's defined in Qiskit. Otherwise it falls back to the custom } After each INSTRUCTION_PARAM the next ``size`` bytes are the parameter's data. -The ``type`` field can be ``'i'``, ``'f'``, ``'p'``, ``'e'``, ``'s'``, ``'c'``, -``'R'`` or ``'n'`` which dictate the format. ``'R'`` is a `REGISTER_PARAM` payload, -identifying a :class:`.ClassicalRegister` or a single :class:`.Clbit` in the encoding described in -:ref:`version 18 `. For ``'i'`` it's an integer, ``'f'`` it's +The ``type`` field can be ``'i'``, ``'f'``, ``'p'``, ``'e'``, ``'s'``, ``'c'`` +or ``'n'`` which dictate the format. For ``'i'`` it's an integer, ``'f'`` it's a double, ``'s'`` if it's a string (encoded as utf8), ``'c'`` is a complex and the data is represented by the struct format in the :ref:`qpy_param_expr` section. ``'p'`` defines a :class:`~qiskit.circuit.Parameter` object which is diff --git a/test/python/qpy/test_roundtrip.py b/test/python/qpy/test_roundtrip.py index 5e40f9464f03..0afe76140657 100644 --- a/test/python/qpy/test_roundtrip.py +++ b/test/python/qpy/test_roundtrip.py @@ -107,68 +107,6 @@ def test_ifelse(self, version, write_with, read_with): qc.if_else(condition, body, false_body, [qc.qubits[0]], []) self.assert_roundtrip_equal(qc, version=version, read_with=read_with, write_with=write_with) - @all_qpy_combinations(QPY_RUST_READ_MIN_VERSION) - def test_ifelse_single_clbit_condition(self, version, write_with, read_with): - """Check an IfElse conditioned on a single clbit rather than a whole register. - - The two share one ``REGISTER`` payload, whose encoding changed in QPY 18, so both arms need - covering: ``test_ifelse`` conditions on a register and this one on a bit. - """ - qc = QuantumCircuit(2, 2) - body = QuantumCircuit([qc.qubits[0]]) - body.x(0) - qc.if_test((qc.clbits[1], True), body, [qc.qubits[0]], []) - self.assert_roundtrip_equal(qc, version=version, read_with=read_with, write_with=write_with) - - @all_qpy_combinations(QPY_RUST_READ_MIN_VERSION) - def test_ifelse_single_clbit_condition_nonzero_index(self, version, write_with, read_with): - qc = QuantumCircuit(3, 4) - body = QuantumCircuit([qc.qubits[0]]) - body.x(0) - - qc.if_test((qc.clbits[2], True), body, [qc.qubits[0]], []) - - self.assert_roundtrip_equal( - qc, - version=version, - read_with=read_with, - write_with=write_with, - ) - - @all_qpy_combinations(QPY_RUST_READ_MIN_VERSION) - def test_ifelse_single_clbit_condition_index_zero(self, version, write_with, read_with): - """Check a bit condition on clbit 0, whose index is the edge case of both encodings. - - Up to QPY 17 it is the shortest possible payload (a null byte and the digit ``0``), and from - QPY 18 it is the only one whose ``uint32_t`` index is entirely zero bytes. - """ - qc = QuantumCircuit(2, 2) - body = QuantumCircuit([qc.qubits[0]]) - body.x(0) - qc.if_test((qc.clbits[0], True), body, [qc.qubits[0]], []) - self.assert_roundtrip_equal(qc, version=version, read_with=read_with, write_with=write_with) - - @all_qpy_combinations(QPY_RUST_READ_MIN_VERSION) - def test_ifelse_condition_among_multiple_registers(self, version, write_with, read_with): - """Check that a condition resolves to the right register when several are present. - - The payload identifies a register by name and a bit by its circuit-wide index, so a circuit - with more than one same-sized register exercises the lookup rather than just the encoding. - """ - qr = QuantumRegister(1, "q") - cregs = [ClassicalRegister(2, name) for name in ("alpha", "beta", "gamma")] - body = QuantumCircuit(1) - body.x(0) - - qc = QuantumCircuit(qr, *cregs) - qc.if_test((cregs[1], 1), body.copy(), [qr[0]], []) - self.assert_roundtrip_equal(qc, version=version, read_with=read_with, write_with=write_with) - - # `gamma[1]` is clbit 5 of the circuit; the payload stores that circuit-wide index. - qc = QuantumCircuit(qr, *cregs) - qc.if_test((cregs[2][1], True), body.copy(), [qr[0]], []) - self.assert_roundtrip_equal(qc, version=version, read_with=read_with, write_with=write_with) - @all_qpy_combinations(QPY_RUST_READ_MIN_VERSION) def test_box(self, version, write_with, read_with): """Check the BoxOp control flow gate passes roundtrip""" diff --git a/test/python/qpy/test_v18.py b/test/python/qpy/test_v18.py index 724ec27f6e0a..15a995dc3683 100644 --- a/test/python/qpy/test_v18.py +++ b/test/python/qpy/test_v18.py @@ -17,8 +17,9 @@ from qiskit.circuit import ClassicalRegister, QuantumCircuit, QuantumRegister from qiskit.circuit.classical import expr -from qiskit.qpy import dump +from qiskit.qpy import dump, load from qiskit.qpy import formats +from qiskit.qpy.exceptions import QpyError from test import QiskitTestCase @@ -82,3 +83,90 @@ def test_switch_case_labels_bytes_differ_v17_vs_v18(self): qc = QuantumCircuit(qr, cr) qc.switch(expr.bit_and(cr, 3), [(1, body.copy()), (2, body.copy())], [0], []) self.assertNotEqual(_dump(qc, 17), _dump(qc, 18)) + + +class TestV18RegisterParam(QiskitTestCase): + """The `Register` payload gained a tag byte in v18. + + It identifies either a whole :class:`.ClassicalRegister` or a single :class:`.Clbit`. Up to v17 + the two shared one untyped string -- a register was its bare name, a clbit was a null byte + followed by its index in ASCII digits -- so the payload could not be identified from its own + bytes. + """ + + #: The condition payload follows the instruction's class name, as these circuits set no label. + GATE_NAME = b"IfElseOp" + + @staticmethod + def _clbit_condition(index=1, num_clbits=2): + """A circuit whose only instruction is conditioned on a single clbit.""" + qreg, creg = QuantumRegister(1, "q"), ClassicalRegister(num_clbits, "creg") + circuit = QuantumCircuit(qreg, creg) + body = QuantumCircuit(1) + body.x(0) + circuit.if_test((creg[index], True), body, [qreg[0]], []) + return circuit + + @staticmethod + def _register_condition(name="creg"): + """A circuit whose only instruction is conditioned on a whole classical register.""" + qreg, creg = QuantumRegister(1, "q"), ClassicalRegister(2, name) + circuit = QuantumCircuit(qreg, creg) + body = QuantumCircuit(1) + body.x(0) + circuit.if_test((creg, 1), body, [qreg[0]], []) + return circuit + + def _condition_payload(self, data, length): + """The ``length`` bytes of condition payload that follow the instruction's class name.""" + start = data.rindex(self.GATE_NAME) + len(self.GATE_NAME) + return bytes(data[start : start + length]) + + def test_clbit_condition_is_tagged_in_v18(self): + """A clbit is a tag byte plus a uint32 index from v18; ASCII digits up to v17.""" + for index in (0, 1, 3): + with self.subTest(index=index): + circuit = self._clbit_condition(index=index, num_clbits=4) + legacy = b"\x00" + str(index).encode("utf8") + self.assertEqual(self._condition_payload(_dump(circuit, 17), len(legacy)), legacy) + tagged = struct.pack( + formats.REGISTER_PARAM_CLBIT_PACK, formats.REGISTER_PARAM_TAG_CLBIT, index + ) + self.assertEqual(self._condition_payload(_dump(circuit, 18), len(tagged)), tagged) + + def test_register_condition_is_tagged_in_v18(self): + """A register is a tag byte plus its name from v18; the bare name up to v17.""" + circuit = self._register_condition() + self.assertEqual(self._condition_payload(_dump(circuit, 17), 4), b"creg") + tagged = ( + struct.pack(formats.REGISTER_PARAM_TAG_PACK, formats.REGISTER_PARAM_TAG_REGISTER) + + b"creg" + ) + self.assertEqual(self._condition_payload(_dump(circuit, 18), len(tagged)), tagged) + + def test_null_prefixed_register_name_needs_v18(self): + """A register name starting with a null byte is only representable from v18. + + Qiskit accepts such a name, but up to v17 it collides with the marker for a single-bit + condition, so the reader takes the name for a bit index and fails. The tag removes the + ambiguity. + """ + circuit = self._register_condition(name="\x00weird") + with self.assertRaises((QpyError, ValueError)): + load(io.BytesIO(_dump(circuit, 17))) + self.assertEqual(load(io.BytesIO(_dump(circuit, 18)))[0], circuit) + + def test_v18_unknown_tag_is_rejected(self): + """An unrecognised tag byte must fail rather than be guessed at.""" + data = bytearray(_dump(self._clbit_condition(), 18)) + data[data.rindex(self.GATE_NAME) + len(self.GATE_NAME)] = 7 + with self.assertRaises(QpyError): + load(io.BytesIO(bytes(data))) + + def test_v18_unknown_register_name_is_rejected(self): + """A register name that is not in the circuit must fail.""" + data = bytearray(_dump(self._register_condition(), 18)) + start = data.rindex(self.GATE_NAME) + len(self.GATE_NAME) + 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)))