Annotation serialisation framework #14439

jakelishman · 2025-05-22T17:16:36Z

This is a roll-up of the current description of the annotation framework. I'll be back later today to finish it up. All of the hard work and design work is done, I just have to finish writing a bit more docs, write the QPY bit of the implementation (which will be small, like the OQ3 implementation, because all the complexity is pushed to users), and the tests (which will spill).

Summary

Implementation list:

Details and comments

coveralls · 2025-05-23T03:13:19Z

Pull Request Test Coverage Report for Build 15451537032

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

For more information on this, see Tracking coverage changes with pull request builds.
To avoid this issue with future PRs, see these Recommended CI Configurations.
For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

222 of 247 (89.88%) changed or added relevant lines in 16 files are covered.
129 unchanged lines in 9 files lost coverage.
Overall coverage increased (+0.06%) to 87.966%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
qiskit/circuit/controlflow/box.py	4	5	80.0%
qiskit/qasm3/exporter.py	9	10	90.0%
qiskit/circuit/annotation.py	43	45	95.56%
qiskit/qasm3/init.py	9	13	69.23%
qiskit/circuit/quantumcircuit.py	10	15	66.67%
qiskit/qpy/binary_io/circuits.py	98	110	89.09%

Files with Coverage Reduction	New Missed Lines	%
crates/qasm2/src/expr.rs	1	94.23%
crates/transpiler/src/passes/unitary_synthesis.rs	1	94.57%
qiskit/qpy/interface.py	1	87.88%
qiskit/transpiler/passes/layout/vf2_utils.py	3	93.33%
crates/qasm2/src/lex.rs	4	91.48%
crates/qasm2/src/parse.rs	6	97.15%
crates/cext/src/exit_codes.rs	10	31.58%
crates/transpiler/src/target/mod.rs	35	84.9%
crates/quantum_info/src/pauli_lindblad_map/qubit_sparse_pauli.rs	68	88.47%

Totals
Change from base Build 15440870635:	0.06%
Covered Lines:	82352
Relevant Lines:	93618

💛 - Coveralls

crates/circuit/src/annotation.rs

kevinhartman

This looks pretty solid to me.

The one concern I have that ties in closely to the control flow oxidation stuff I'm working on is the Rust-side representation of annotations. We're going to need to use a Py<PyAny> to represent these, and since you've got them as fields of BoxOp as opposed to params, they'll end up roughly here, directly on the ControlFlow operation enum: OperationRef::ControlFlow(ControlFlow::Box { annotations, ... }). At the moment, ControlFlow derives PartialEq, but with a Py<PyAny> inside, we'd have to implement it manually and acquire the GIL (if we want it implemented at all).

If you stick it into params, that shouldn't be an issue since we already compare those with the Python token. I've got a ControlFlowView<'a> enum FWIW that ties together what we store in the operation (ControlFlow) and the params into a unified view:

pub enum ControlFlowView<'a, T> {
    Box(Option<&'a Duration>, &'a T),
    BreakLoop,
    ContinueLoop,
    ForLoop {
        indexset: &'a [usize],
        loop_param: Option<&'a PyObject>,
        body: &'a T,
    },
    IfElse {
        condition: &'a Condition,
        true_body: &'a T,
        false_body: Option<&'a T>,
    },
    Switch {
        target: &'a Target,
        cases_specifier: Vec<(&'a Vec<CaseSpecifier>, &'a T)>,
    },
    While {
        condition: &'a Condition,
        body: &'a T,
    },
}

Separately, I'm curious how you envision annotations being exposed to Rust in the future. Any thoughts there?

I'll have another look once you've got the tests and remaining documentation TODOs sorted.

qiskit/circuit/annotation.py

kevinhartman · 2025-06-02T19:42:36Z

qiskit/circuit/quantumcircuit.py

                qubits,
                clbits,
                copy=False,
            )
+        if body_or_annotations is ...:
+            annotations = () if annotations is ... else annotations


Very minor, but should this be [] instead of ()? Since I think we call list on the annotations coming in to the BoxOp.

list always creates a new list from its input, so it doesn't make much difference, and has a specialised fast path for conversion from tuple. The empty tuple is a singleton in CPython, so actually this is probably slightly faster because it avoids an intermediate allocation.

also, I probably originally used () as the kwarg default (since it's immutable and safe, unlike []), then realised that I needed to be able to tell if the object had been passed or not, and () is a valid value

kevinhartman · 2025-06-03T15:00:01Z

qiskit/circuit/annotation.py

+            namespace: the namespace that this serializer was accessed under.  This may be an
+                ancestor of the annotation.


Just curious, what would the expected use case for this be? Would an implementation decide to return NotImplemented in some case based on this, or would we expect this to be part of the payload they write out?

The idea of namespaces having .-separated identifiers is just straight from the OQ3 spec, and then I leant into it in the rest of the design. It's nice, especially in conjunction with the serialisation state in QPY, because you can have a single serialiser object that can handle multiple different types of annotation, and use a shared state for all of them. It gives you a framework for having multiple Annotation subclasses that share some higher logical state (like references to pulse channels or noise models, maybe), so you only want a single serialiser, and the namespace lets you distinguish things.

The NotImplemented thing came from when I was intending to have a catch-all serialiser for QPY that looked things up using globally-configured OpenQASM 3 ones, and so the exception is managed as a proper type state during serialisation/deserialisation, which will make it rather cheaper to handle when moving down to Rust. I didn't want that to be a special case, so I just said it's available at all levels of the lookup - I can imagine a situation where you might have a "specialised" serialiser that handles only some entries in a dotted namespace, and you want a general serialiser for the whole namespace as a backup.

qiskit/circuit/quantumcircuit.py

kevinhartman · 2025-06-03T15:13:28Z

qiskit/qasm3/exporter.py

+        """Use the custom serializers to construct an annotation object."""
+        for namespace in iter_namespaces(annotation.namespace):
+            if (serializer := self.annotation_serializers.get(namespace, None)) is not None and (
+                payload := serializer.dump(annotation)


Are there any checks we should do on the payload to make sure it's supported by QASM? Disclaimer: I haven't looked at the QASM spec to see what's supported.

It just has to be a single line of valid utf-8 text. Python should enforce valid utf-8 by nature of the return value being str, and I just didn't include the check for \n since imo that'd be something that should live in the test suites of the custom serialisers.

kevinhartman · 2025-06-03T15:19:46Z

qiskit/qpy/__init__.py

+    struct INSTRUCTION;
+    uint8_t name[name_size];
+    uint8_t label[label_size];
+    uint8_t register[conditional_reg_name_size]; (1)


Should this (1) be after struct INSTRUCTION instead?

I'd intended it to be a note read as part of understanding the line it's actually on - I can say from just having used the note myself during debugging that it at least made sense to me haha. I can see the argument for it being on the struct INSTRUCTION line if you prefer.

kevinhartman · 2025-06-03T15:37:58Z

qiskit/qpy/binary_io/circuits.py

+            if (serializer := self.potential_serializers.get(namespace, None)) is not None:
+                if (out := serializer.dump_annotation(annotation)) is not NotImplemented:
+                    del self.potential_serializers[namespace]
+                    index = len(self.serializers)


Should we have size checking on the number of serializers to make sure we don't exceed the width of a u32? I don't think we do that for anything else in QPY though, anyway.

that's like 4 billion or something - I think you'd have larger problems if you were exceeding that

jakelishman · 2025-06-03T16:39:51Z

Longer term, I want annotation to be a property of PackedInstruction not a special behaviour of box (I think I wrote that somewhere in the docs), and to be the general way that we do tagging of temporary local inter-pass attributes onto single instructions inside the compiler IR, so various passes don't try (poorly) to reinvent the wheel, and so the pass manager can reason about them in a limited way. That also happens to match how OQ3 handles annotations. I just stuck to only Box for the first iteration, which is where it needs to be used at first.

In Rust, I will probably want to hide the list of annotations in the sort of "extra attributes" indirection we used to have for condition/unit/duration/label, and then got reduced down just to label, because we expect the use of it to be rare relative to instructions.

As to representation of Annotation in Rust itself: my approximate idea is that there will be a pretty limited "properties" API that annotations have to specify how they behave under certain very abstract IR manipulations (valid under co-variant transformations of types X, Y Z; valid under contra-variant transformations; valid only for the verbatim set of instructions at the time of tagging; etc, where the "transformations" are things like "the physical qubits used", "the physical qubit 2q connections used", etc). The results of those configurations will all be Rust native, accessed by some sort of virtual-dispatch trait method or something. All the custom data of annotations will be custom and generally invisible; only the passes intended to interpret a given annotation (can use the namespace to determine, perhaps) will know what type to interpret the pointer as, and thus to use it, etc.

I don't have the full Rust implementation fleshed out - it's very similar to the problem about how we'll represent custom gates in a way that isn't Python-specific, so e.g. Rust (including Qiskit itself) can put temporary custom objects into the circuit. I have rough ideas, but for the time being, I'm just throwing it into the "sort out with the other custom stuff" bucket, since annotations are needed by the primitives teams in the short term.

This adds the basics of support for a custom annotation framework to Qiskit. An `Annotation` is a piece of custom-defined information (from Qiskit's perspective; it's most likely to be transpiler-pass defined, even downstream) that is localised to a particular instruction or scope. Currently, the only supported location for use of annotations is `box`. The meaning of the `Annotation` may currently be completely custom. We expect in the future to have a coarse set of validity ranges that annotations associate themselves with, to allow the transpiler to enforce that annotations are not violated by compiler passes that do not understand them. For now, it's up to the user not to call compiler passes that invalidate annotations. Annotations are closely related to the OpenQASM 3 concept of the same name. Serialisation to and from OpenQASM 3 and QPY is supported by additional user-defined classes, whose interfaces are defined in `qiskit.circuit.annotation`. QPY version 15 is introduced, which makes it a collaborative serialisation format, with well-defined slots in the output stream for custom serialisers to store their data, including state. OpenQASM 3 annotation serialisers must be stateless, since that language does not provide (nor intend) the same arbitrary co-operation. QPY has this additional support for statefulness because we anticipate larger amounts of data being transferred in annotations in QPY format for subsequent processing.

jakelishman · 2025-06-03T20:28:16Z

Ok, that should be the last time I force push to this branch - I'll just do everything additively from now. The major changes in the last force push were to modify how the QPY serialiser subclasses work - previously they were passing a namespace to all four of the methods, but actually, the way the interface is designed, it was only possible for a single look-up key to be associated with any one instance, so half of the passing was irrelevant.

As a note: the current implementation makes a new annotation-state context for each subcircuit; we don't share the annotation state when we enter a Box, for example, so annotations in a nested box will be serialised into a clean state. This is consistent with how the same registers will end up getting re-serialised, etc, within control-flow blocks in QPY. In a future version of QPY, we should make control flow more natively supported, and have the subcircuits share the same annotation contexts (and register contexts, Var contexts, etc), since that'll be more efficient for serialisation.

jakelishman · 2025-06-04T00:01:41Z

Qiskit/qiskit-qasm3-import#50 adds an implementation of annotated boxes, and provides an API that this library can then wrap. I tested that PR against this branch.

This does a small amount of version negotiation with `qiskit_qasm3_import`, which in turn checks the version of Qiskit.

ElePT · 2025-06-04T13:54:42Z

qiskit/qpy/__init__.py

+Changes within PARAM_EXPR_ELEM_V13
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+The struct itself is unchanged.  However, for a ``PARAM_EXPR_ELEM_V13`` representing a
+:meth:`.ParameterExpression.subs` call (with ``op_code = 15``, and therefore ``lhs_type = 'p'`` and
+``rhs_type = 'n'``), the trailing :ref:`qpy_mapping` now maps keys of the raw bytes of the
+:class:`.Parameter` UUIDs to the substituted values.  Previously (in QPY versions 13 and 14), this
+mapping stored the parameter names as the keys.
+


This looks much better than the original wording. It's also nice to have more changes that justify a QPY version change, instead of a tiny detail

qiskit-bot · 2025-06-04T15:50:26Z

One or more of the following people are relevant to this code:

@Eric-Arellano
@abbycross
@beckykd
@Qiskit/terra-core
@mtreinish
@nkanazawa1989

abbycross · 2025-06-04T16:07:38Z

test/python/circuit/test_annotation.py

+from qiskit.circuit import annotation
+from test import QiskitTestCase  # pylint: disable=wrong-import-order
+
+# Most of the tests for this module are elsewhere, such as for OpenQASM 3 serialisation being in the


Suggested change

# Most of the tests for this module are elsewhere, such as for OpenQASM 3 serialisation being in the

# Most of the tests for this module are elsewhere, such as for OpenQASM 3 serialization being in the

Thanks Abby - this particular one is just a code comment, and you're fighting a losing battle trying to localise those (I do my best in public docs, but there's a limit when it's just me "talking" in comments).

I hear that! Copyediting is a constant losing battle, so I'm used to that :D Forgive me when I made such edits, but the American spelling per IBM Style guidance is branded in my brain.

abbycross · 2025-06-04T16:07:51Z

test/python/qasm3/test_export.py

@@ -3139,3 +3139,102 @@ def test_old_alias_classical_registers_option(self):
            ]
        )
        self.assertEqual(dumps_experimental(qc, allow_aliasing=True), expected_qasm)
+
+    def test_annotations(self):
+        """Test that the annotation-serialisation framework works."""


Suggested change

"""Test that the annotation-serialisation framework works."""

"""Test that the annotation-serialization framework works."""

abbycross · 2025-06-04T16:08:01Z

test/python/qpy/test_circuit_load_from_qpy.py

+        triggered_not_implemented = False
+
+        class Serializer(annotation.QPYSerializer):
+            # The idea with this family of serialisers is that we say the "value" we expect to see


Suggested change

# The idea with this family of serialisers is that we say the "value" we expect to see

# The idea with this family of serializers is that we say the "value" we expect to see

jakelishman · 2025-06-04T16:17:23Z

Abby: thanks for the effort with the localisation. In general, code comments (starting # or //) don't need localising, and even docstrings in files that start test/ generally don't either - neither of those are publicly visible at all.

I try my best with the spelling when I'm writing public-facing documentation, but it's a (small) effort for me that I just don't bother with when it's not for publication - most of our test docstrings and code comments are barely even grammatically correct...

If I were you, I wouldn't waste time looking at those. Comments on the public ones (docstrings in qiskit/ or crates/) are very welcome though.

qiskit/qasm3/exporter.py

kevinhartman

This looks good to me, thanks! Just one minor comment in the tests.

Longer term, I want annotation to be a property of PackedInstruction not a special behaviour of box
...
In Rust, I will probably want to hide the list of annotations in the sort of "extra attributes" indirection we used to have for condition/unit/duration/label, and then got reduced down just to label, because we expect the use of it to be rare relative to instructions.

Ah alright, that sounds good then. I would've imagined the same regarding how we'd store them on PackedInstruction.

The other context you've provided on the future of this in Rust is helpful.

test/python/qasm3/test_export.py

kevinhartman

LGTM!

* Add `Annotation` base class and `BoxOp` support This adds the basics of support for a custom annotation framework to Qiskit. An `Annotation` is a piece of custom-defined information (from Qiskit's perspective; it's most likely to be transpiler-pass defined, even downstream) that is localised to a particular instruction or scope. Currently, the only supported location for use of annotations is `box`. The meaning of the `Annotation` may currently be completely custom. We expect in the future to have a coarse set of validity ranges that annotations associate themselves with, to allow the transpiler to enforce that annotations are not violated by compiler passes that do not understand them. For now, it's up to the user not to call compiler passes that invalidate annotations. Annotations are closely related to the OpenQASM 3 concept of the same name. Serialisation to and from OpenQASM 3 and QPY is supported by additional user-defined classes, whose interfaces are defined in `qiskit.circuit.annotation`. QPY version 15 is introduced, which makes it a collaborative serialisation format, with well-defined slots in the output stream for custom serialisers to store their data, including state. OpenQASM 3 annotation serialisers must be stateless, since that language does not provide (nor intend) the same arbitrary co-operation. QPY has this additional support for statefulness because we anticipate larger amounts of data being transferred in annotations in QPY format for subsequent processing. * Complete documentation of subclassing * Fix circuit equality of boxes * Add support for annotations in `qasm3.load` API This does a small amount of version negotiation with `qiskit_qasm3_import`, which in turn checks the version of Qiskit. * Include comment on equality * Add OpenQASM 3 import/export tests * Fix previous parameter-expression QPY documentation * Support `BoxOp` in `GenericBackendV2` * Fix annotation handling in `BoxOp.replace_blocks` * Add transpilation integration test * Fix GenericBackendV2 test * Add `iter_namespaces` test * Add QPY tests * Add release note * Fix parameter documentation * Make test logic errors exceptions

mtreinish added this to the 2.1.0 milestone May 22, 2025

mtreinish added priority: high Changelog: New Feature Include in the "Added" section of the changelog labels May 22, 2025

jakelishman force-pushed the box/annotation branch 2 times, most recently from d465b75 to a09da46 Compare May 23, 2025 02:31

alexanderivrii reviewed May 23, 2025

View reviewed changes

crates/circuit/src/annotation.rs Outdated Show resolved Hide resolved

eliarbel assigned kevinhartman and jakelishman May 27, 2025

jakelishman force-pushed the box/annotation branch 2 times, most recently from 9ef71a7 to e0ce5f0 Compare June 2, 2025 13:31

kevinhartman reviewed Jun 3, 2025

View reviewed changes

jakelishman force-pushed the box/annotation branch from e0ce5f0 to 383a676 Compare June 3, 2025 19:23

jakelishman changed the title ~~WIP: Annotation serialisation framework~~ Annotation serialisation framework Jun 3, 2025

jakelishman force-pushed the box/annotation branch from 383a676 to 6339ff5 Compare June 3, 2025 19:38

jakelishman added 2 commits June 3, 2025 22:27

Complete documentation of subclassing

ad529e6

Fix circuit equality of boxes

a06a6f1

jakelishman mentioned this pull request Jun 3, 2025

Add support for annotated boxes in Qiskit 2.1 Qiskit/qiskit-qasm3-import#50

Merged

jakelishman added 5 commits June 4, 2025 12:13

Add support for annotations in qasm3.load API

ea24c44

This does a small amount of version negotiation with `qiskit_qasm3_import`, which in turn checks the version of Qiskit.

Include comment on equality

3d59023

Add OpenQASM 3 import/export tests

8ec17c3

Merge remote-tracking branch 'ibm/main' into box/annotation

09db88b

Fix previous parameter-expression QPY documentation

02a3879

ElePT reviewed Jun 4, 2025

View reviewed changes

jakelishman added 2 commits June 4, 2025 15:35

Support BoxOp in GenericBackendV2

e1aec18

Fix annotation handling in BoxOp.replace_blocks

e98f9f4

jakelishman added 4 commits June 4, 2025 15:41

Add transpilation integration test

d2edc73

Fix GenericBackendV2 test

707bb6f

Add iter_namespaces test

7d06772

Add QPY tests

9e8dfa2

jakelishman marked this pull request as ready for review June 4, 2025 15:50

jakelishman requested review from jyu00 and a team as code owners June 4, 2025 15:50

Add release note

f112f29

abbycross reviewed Jun 4, 2025

View reviewed changes

jakelishman commented Jun 4, 2025

View reviewed changes

qiskit/qasm3/exporter.py Outdated Show resolved Hide resolved

Fix parameter documentation

722df62

kevinhartman reviewed Jun 4, 2025

View reviewed changes

test/python/qasm3/test_export.py Outdated Show resolved Hide resolved

Make test logic errors exceptions

572baf0

kevinhartman approved these changes Jun 4, 2025

View reviewed changes

kevinhartman enabled auto-merge June 4, 2025 20:07

kevinhartman added this pull request to the merge queue Jun 4, 2025

Merged via the queue into Qiskit:main with commit a357f54 Jun 4, 2025
26 checks passed

jakelishman deleted the box/annotation branch June 4, 2025 20:52

jakelishman mentioned this pull request Jun 4, 2025

Add box-annotation framework to transpiler #13773

Closed

		namespace: the namespace that this serializer was accessed under. This may be an
		ancestor of the annotation.

	# Most of the tests for this module are elsewhere, such as for OpenQASM 3 serialisation being in the
	# Most of the tests for this module are elsewhere, such as for OpenQASM 3 serialization being in the

	"""Test that the annotation-serialisation framework works."""
	"""Test that the annotation-serialization framework works."""

	# The idea with this family of serialisers is that we say the "value" we expect to see
	# The idea with this family of serializers is that we say the "value" we expect to see

Annotation serialisation framework #14439

Annotation serialisation framework #14439

Uh oh!

Conversation

jakelishman commented May 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Details and comments

Uh oh!

coveralls commented May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 15451537032

Warning: This coverage report may be inaccurate.

Details

💛 - Coveralls

Uh oh!

Uh oh!

kevinhartman left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jakelishman commented Jun 3, 2025

Uh oh!

jakelishman commented Jun 3, 2025

Uh oh!

jakelishman commented Jun 4, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qiskit-bot commented Jun 4, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jakelishman commented Jun 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

kevinhartman left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kevinhartman left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jakelishman commented May 22, 2025 •

edited

Loading

coveralls commented May 23, 2025 •

edited

Loading

jakelishman commented Jun 4, 2025 •

edited

Loading