Fix annotation-handling in Rust-space QPY (#15649)

* Fix annotation-handling in Rust-space QPY

The Rust-space QPY serialiser was not actually serialising the
annotations successfully; it always returned zero annotations due to an
accidental overwrite of the `CircuitInstructionV2Pack::annotations`
field, which was not updated when control flow operations moved to Rust.
Unfortunately, the round-trip tests also did not consider annotations,
because annotations, in general, do not need to have equality defined
between them.

This corrects the annotations implementation (it was mostly already
correct), and fixes the equality checks in the tests.

* Remove useless f-strings

Author: jakelishman

Cargo.lock

@@ -110,7 +110,10 @@ impl<'a> AnnotationHandler<'a> {
                     }
                 }
             }
-            Ok((0, Bytes(Vec::new())))
+            Err(QpyError::AnnotationError(format!(
+                "No configured annotation serializer could handle {}",
+                annotation.bind(py).repr()?,
+            )))
         })
     }
 

crates/qpy/src/annotations.rs

@@ -47,8 +47,8 @@ use crate::formats;
 use crate::params::pack_param_obj;
 use crate::py_methods::{
     PAULI_PRODUCT_MEASUREMENT_GATE_CLASS_NAME, PAULI_PRODUCT_ROTATION_GATE_CLASS_NAME,
-    UNITARY_GATE_CLASS_NAME, gate_class_name, getattr_or_none, py_get_instruction_annotations,
-    py_pack_param, py_pack_pauli_evolution_gate, recognize_custom_operation, serialize_metadata,
+    UNITARY_GATE_CLASS_NAME, gate_class_name, getattr_or_none, py_pack_param,
+    py_pack_pauli_evolution_gate, recognize_custom_operation, serialize_metadata,
 };
 use crate::value::{
     BitType, CircuitInstructionType, ExpressionVarDeclaration, GenericValue, ParamRegisterValue,
@@ -250,7 +250,6 @@ fn pack_instruction(
         instruction_pack.label = label.clone();
     }
     instruction_pack.bit_data = get_packed_bit_list(instruction, qpy_data.circuit_data);
-    instruction_pack.annotations = py_get_instruction_annotations(instruction, qpy_data)?;
     if let Some(new_name) =
         recognize_custom_operation(&instruction.op, &gate_class_name(&instruction.op)?)?
     {
@@ -382,8 +381,6 @@ fn pack_control_flow_inst(
 ) -> Result<formats::CircuitInstructionV2Pack, QpyError> {
     let mut packed_annotations = None;
     let mut packed_condition: formats::ConditionPack = Default::default();
-    let mut extras_key = 0; // should contain a combination of condition key and annotations key, if present
-
     let params = match control_flow_inst.control_flow.clone() {
         ControlFlow::Box {
             duration,
@@ -420,12 +417,10 @@ fn pack_control_flow_inst(
         }
         ControlFlow::IfElse { condition } => {
             packed_condition = pack_condition(condition, qpy_data)?;
-            extras_key = packed_condition.key() as u8;
             pack_instruction_blocks(instruction, qpy_data)?
         }
         ControlFlow::While { condition } => {
             packed_condition = pack_condition(condition, qpy_data)?;
-            extras_key = packed_condition.key() as u8;
             pack_instruction_blocks(instruction, qpy_data)?
         }
         ControlFlow::Switch {
@@ -467,10 +462,16 @@ fn pack_control_flow_inst(
             params
         }
     };
+    let annotations_key = if packed_annotations.is_some() {
+        formats::extras_key_parts::ANNOTATIONS
+    } else {
+        0
+    };
+    let condition_key = packed_condition.key() as u8;
     Ok(formats::CircuitInstructionV2Pack {
         num_qargs: control_flow_inst.num_qubits,
         num_cargs: control_flow_inst.num_clbits,
-        extras_key,
+        extras_key: condition_key | annotations_key,
         num_ctrl_qubits: 0, // standard instructions have no control qubits
         ctrl_state: 0,
         gate_class_name: control_flow_inst.name().to_string(), // this name is NOT a proper python class name, but we don't instantiate from the python class anymore

crates/qpy/src/circuit_writer.rs

@@ -132,6 +132,10 @@ pub enum QpyError {
     #[error("binary parsing error: {0}")]
     BinRwError(String),
 
+    /// Something went wrong in annotation handling.
+    #[error("annotation-handling error: {0}")]
+    AnnotationError(String),
+
     /// Python error that occurred during a Python call
     #[error("Python error: {0}")]
     PythonError(#[from] PyErr),

crates/qpy/src/error.rs

@@ -16,9 +16,7 @@ use numpy::Complex64;
 use pyo3::IntoPyObjectExt;
 use pyo3::intern;
 use pyo3::prelude::*;
-use pyo3::types::{
-    PyAny, PyComplex, PyDict, PyFloat, PyInt, PyIterator, PyList, PyString, PyTuple,
-};
+use pyo3::types::{PyAny, PyComplex, PyDict, PyFloat, PyInt, PyList, PyString, PyTuple};
 use qiskit_circuit::classical::expr::Expr;
 use std::num::NonZero;
 use std::sync::Arc;
@@ -27,7 +25,7 @@ use qiskit_circuit::bit::{ClassicalRegister, ShareableClbit};
 use qiskit_circuit::classical;
 use qiskit_circuit::imports;
 use qiskit_circuit::operations::{Operation, OperationRef, PyRange};
-use qiskit_circuit::packed_instruction::{PackedInstruction, PackedOperation};
+use qiskit_circuit::packed_instruction::PackedOperation;
 use qiskit_circuit::parameter::parameter_expression::{
     PyParameter, PyParameterExpression, PyParameterVectorElement,
 };
@@ -513,35 +511,6 @@ pub(crate) fn py_pack_param(
     Ok(formats::GenericDataPack { type_key, data })
 }
 
-pub(crate) fn py_get_instruction_annotations(
-    instruction: &PackedInstruction,
-    qpy_data: &mut QPYWriteData,
-) -> Result<Option<formats::InstructionsAnnotationPack>, QpyError> {
-    Python::attach(|py| {
-        if let OperationRef::Instruction(inst) = instruction.op.view() {
-            let op = inst.instruction.bind(py);
-            if op.is_instance(imports::CONTROL_FLOW_BOX_OP.get_bound(py))? {
-                let annotations_iter = PyIterator::from_object(&op.getattr("annotations")?)?;
-                let annotations: Vec<formats::InstructionAnnotationPack> = annotations_iter
-                    .map(|annotation| {
-                        let (namespace_index, payload) = qpy_data
-                            .annotation_handler
-                            .serialize(&annotation?.unbind())?;
-                        Ok(formats::InstructionAnnotationPack {
-                            namespace_index,
-                            payload,
-                        })
-                    })
-                    .collect::<Result<_, QpyError>>()?;
-                if !annotations.is_empty() {
-                    return Ok(Some(formats::InstructionsAnnotationPack { annotations }));
-                }
-            }
-        }
-        Ok(None)
-    })
-}
-
 pub(crate) fn py_pack_modifier(modifier: &Py<PyAny>) -> Result<formats::ModifierPack, QpyError> {
     Python::attach(|py| {
         let modifier = modifier.bind(py);

crates/qpy/src/py_methods.rs

@@ -21,7 +21,7 @@
 from qiskit.circuit.random import random_circuit
 from qiskit.providers.fake_provider import GenericBackendV2
 from qiskit.exceptions import QiskitError
-from qiskit.qpy import dump, load, formats, get_qpy_version, QPY_COMPATIBILITY_VERSION
+from qiskit.qpy import dump, load, formats, get_qpy_version, QPY_COMPATIBILITY_VERSION, QpyError
 from qiskit.qpy.common import QPY_VERSION
 from qiskit.transpiler import TranspileLayout, CouplingMap
 from qiskit.compiler import transpile
@@ -60,6 +60,18 @@ def assert_roundtrip_equal(
                 file_version,
                 f"Generated QPY file version {file_version} does not match request version {version}",
             )
+        if annotation_factories is not None:
+
+            def flat_annotations(qc):
+                for inst in qc.data:
+                    op = inst.operation
+                    if inst.name == "box":
+                        yield op.annotations
+                    if inst.is_control_flow():
+                        for block in op.blocks:
+                            yield from flat_annotations(block)
+
+            self.assertEqual(list(flat_annotations(circuit)), list(flat_annotations(new_circuit)))
 
 
 class TestVersions(QpyCircuitTestCase):
@@ -398,6 +410,9 @@ def __eq__(self, other):
                     and self.value == other.value
                 )
 
+            def __repr__(self):
+                return f"My({self.namespace!r}, {self.value!r})"
+
         class Serializer(annotation.OpenQASM3Serializer):
             def load(self, namespace, payload):
                 return My(namespace, payload)
@@ -420,6 +435,9 @@ class Dummy(annotation.Annotation):
             def __eq__(self, other):
                 return isinstance(other, Dummy)
 
+            def __repr__(self):
+                return "Dummy()"
+
         class Serializer(annotation.QPYSerializer):
             def load_annotation(self, payload):
                 outer_self.assertEqual(payload, b"SENTINEL")
@@ -446,6 +464,9 @@ def __init__(self, value):
             def __eq__(self, other):
                 return isinstance(other, My) and self.value == other.value
 
+            def __repr__(self):
+                return f"My({self.value!r})"
+
         class Serializer(annotation.QPYSerializer):
             def __init__(self):
                 self.loaded_state = False
@@ -494,12 +515,18 @@ class TypeA(annotation.Annotation):
             def __eq__(self, other):
                 return isinstance(other, TypeA)
 
+            def __repr__(self):
+                return "TypeA()"
+
         class TypeB(annotation.Annotation):
             namespace = "b"
 
             def __eq__(self, other):
                 return isinstance(other, TypeB)
 
+            def __repr__(self):
+                return "TypeB()"
+
         class SerializerA(annotation.QPYSerializer):
             def dump_annotation(self, namespace, annotation):
                 outer_self.assertEqual(namespace, "a")
@@ -558,6 +585,9 @@ def __eq__(self, other):
                     and self.value == other.value
                 )
 
+            def __repr__(self):
+                return f"My({self.namespace!r}, {self.value!r})"
+
         triggered_not_implemented = False
 
         class Serializer(annotation.QPYSerializer):
@@ -608,6 +638,17 @@ def load_state(self, namespace, payload):
         )
         self.assertTrue(triggered_not_implemented)
 
+    def test_reject_unknown_namespace(self):
+        class Unknown(annotation.Annotation):
+            namespace = "unknown"
+
+        qc = QuantumCircuit()
+        with qc.box([Unknown()]):
+            pass
+
+        with io.BytesIO() as fptr, self.assertRaisesRegex(QpyError, "No configured annotation"):
+            dump(qc, fptr)
+
 
 @ddt
 class TestOutputStreamProperties(QpyCircuitTestCase):