QPY Rust and Python compatibility fixes by gadial · Pull Request #15847 · Qiskit/qiskit

gadial · 2026-03-22T10:10:30Z

Summary

Fixes several incompatibilities between the Python and Rust file formats; in all cases, the Rust code was changed to conform to the Python expected standard

Details and comments

This PR handles several problems detected with the Rust QPY component, most related to problems arising when an older, Python-based version of the QPY module attempts to read files generated with the Rust module. New tests were added to cover all these cases.

ParameterVector in qpy_reply

In Python, the data type of a ParameterVector inside the qpy_replay object is stored as Parameter (b'p') and not ParameterVector (b'v'). Rust saved them with separate types, resulting in Python failing to load QPY files generated with Rust when the circuits contained parameter expressions involving ParameterVectors.

The fix makes Rust save the data type as Parameter as well. This should not cause problems, as parameters and parameter vectors are treated exactly the same when unpacking a parameter expression:

let lhs: Option<ParameterValueType> = match op.lhs_type {
                ParameterType::Parameter | ParameterType::ParameterVector => {
                    if let Some(value) = param_uuid_map.get(&op.lhs) {
                        Some(parameter_value_type_from_generic_value(value)?)
                    } else {

Note that writing `b'v' here breaks the QPY17 format, meaning that version 2.4.0rc1 does not fully conform to QPY17.

BoxOp duration parameter

In Rust, the BoxOp duration parameter was stored as GenericData which is either duration or expression. This should be the implementation going forward to QPY18 and beyond, but in Python support for duration in instruction parameters was nonexistant, so a hack was used instead, where BoxOp had two parameters, one for the numerical value and the other for the string of the duration's units - as well as "expr" when storing an expression. The current Rust code now conforms to this hack.

Control flow names

Previously, Rust stored the "new style" control flow names, e.g. if_else. This makes Python unable to correctly create those gates as it expects the actual class name. A conversion method from the new to the old style was added.

SwitchCaseOp handling

The way in which SwitchCaseOp parameters are stored in Python QPY is obsolete since the introduction of Rust based control flow ops. However, for now we still need to conform to the old method and the code was changed to reflect this. This allows us to dispose of the specialized treatment for this case in unpack_py_instruction since now we won't read switch instructions via this path.

Since the qpy write code of 2.4.0rc1 already used the new (now reverted) style that does not conform to QPY17 (although control flows are not defined in the spec), but we'll probably transition to it on QPY18, the read code now supports both approaches.

Additional bugfixes

As part of the new testing, another bug was discovered in the way condition register is handled - if its value is 0, the program would raise an error. This was fixed.

Another problem discovered was related to parameter vector name lookup in the old style using name-hashing (which was dumped in QPY15). For a parameter vector element e.g. rz[0] the name used was rz while the lookup tried to find rz. This was fixed and the correct name is now stored. However, I did not manage to reproduce the error even with the QPY which led to it being reported. The user that raised the issue confirmed this PR solved it for him.

…e Python module expects

qiskit-bot · 2026-03-22T10:10:35Z

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

@Qiskit/terra-core
@mtreinish

Cryoris

I don't quite understand between which versions this PR fixes compatibility issues. Isn't V17 always loaded by Rust already?

Cryoris · 2026-03-23T07:46:58Z

crates/qpy/src/circuit_writer.rs

@@ -163,9 +163,8 @@ fn pack_condition(
            let register_size = bytes.len() as u16;
            let data = formats::ConditionData::Register(bytes);
            // TODO: this may cause loss of data, but we are constrained by the current qpy format


Is this comment still valid or is it resolved with the 0-default value?

No, it's unrelated. The potential loss of data is when the value is too large. We do have BigUint support in the module, it's simply that V17 doesn't use it at this point.

Cryoris · 2026-03-23T07:48:36Z

crates/qpy/src/params.rs

            (ParameterType::Parameter, *parameter.symbol.uuid.as_bytes())
        }
        ParameterValueType::VectorElement(element) => (
-            ParameterType::ParameterVector,


I'm a bit surprised the ParameterVector type is not needed for anything -- why did we introduce it then? 🤔 The one additional info that a parameter vector element has is the index, was it connected to that?

It is needed in general, but in that specific point of the replay there's no real difference between it and Parameter.

More specifically: this is a completely different part of the QPY spec to the bit that specifies Parameter, it just happens to use the same magic sigil (p). There's no need to distinguish what subclass of Parameter it is, because this part of the format doesn't store all the information about a Parameter anyway - it stores the UUID and the rebuilder has to retrieve the complete Parameter or ParameterVector from a running map of UUID: Parameter that it keeps.

crates/qpy/src/circuit_writer.rs

gadial · 2026-03-23T10:36:49Z

I don't quite understand between which versions this PR fixes compatibility issues. Isn't V17 always loaded by Rust already?

The problem is when writing V17 via Rust then reading it via Python. This path wasn't tested enough, as opposed to the other direction (writing in Python, reading in Rust). The main problem is regarding how control flows are handled; in Rust the parameters were written differently and Python didn't know how to handle them. This breaks V17 and so must be fixed.

qiskit-bot · 2026-03-24T10:05:24Z

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

@Qiskit/terra-core
@mtreinish

crates/qpy/src/circuit_writer.rs

eliarbel · 2026-03-25T09:18:27Z

crates/qpy/src/circuit_reader.rs

+                | "ForLoopOp"
+                | "BreakLoopOp"
+                | "ContinueLoopOp"
+                | "SwitchCaseOp"


I may be missing something here, but don't we need BoxOp as well? And related to this, what is the difference between unpack_control_flow and unpack_py_instruction when it comes to handling control flow ops?

unpack_control_flow is Rust pure whereas unpack_py_instruction creates a python instruction by using a python constructor (which in turn, might be simply rust code accessed through a wrapper). We still use unpack_py_instruction to handle BoxOp instructions (this can be changed, but I think it's out of scope for this PR).

eliarbel · 2026-03-26T10:13:42Z

crates/qpy/src/circuit_reader.rs

-                    )
-                })?;
-            let target = match target_value {
+            let (target_value, case_label_list) = if instruction_values.len() < 3 {


Don't we want here exactly 2 values (params)?

We do, and if we don't have them - there's an error being raised down the line.

The following pattern matching is also written as "at least 2 items in the slice". I think it would be better to spell this out explicitly. But any way, not worth blocking on this, we can handle later.

crates/qpy/src/circuit_reader.rs

eliarbel · 2026-03-26T10:16:30Z

crates/qpy/src/circuit_reader.rs

+                }
+                (target_value, case_label_list)
+            } else {
+                param_values = instruction_values.split_off(3);


With the updated pack_control_flow_inst, can we get here?

No, but this is the way to parse the faulty 2.4.0rc1 files (and pass backwards compatibility). In QPY18 this should be the way to write/read, in my opinion, since it's closer in spirit to how the data is stored, rust-side.

Ok, yeah, I thought this is something for a future format. NP, we can keep this.

Let's take a follow-on action item to either insert a comment explaining that this path should only be taken by the to-be-yanked 2.4.0rc1, or to delete the code.

Co-authored-by: Eli Arbel <46826214+eliarbel@users.noreply.github.com>

Cryoris

Some implementation comments, though this now mostly looks clear, thanks! I'll play with this now and try and break it 🙂

Cryoris · 2026-03-26T13:58:46Z

crates/qpy/src/value.rs

        }
    }
+    // return the inner Vec when the GenericData is a Tuple
+    pub(crate) fn as_vec(&self) -> Option<&Vec<GenericValue>> {


Using as_slice and returning Option<&[GenericValue]> seems more Rust idiomatic here

Still not fluent enough in Rust to naturally think of doing it... fixed in f4e4391

Cryoris · 2026-03-26T14:03:00Z

crates/qpy/src/circuit_writer.rs

-            let low_digits = target_value.iter_u64_digits().next().ok_or_else(|| {
-                QpyError::MissingData("Register condition value is missing".to_string())
-            })? as i64;
+            if target_value > BigUint::from(i64::MAX as u64) {


Not urgent, but do we have a test triggering this?

No. I created one manually but didn't want to add it to the test suite since we don't have a standard across QPY versions how to deal with this situation (in Python it crashed ungracefully). I can add if we want to adopt the Rust error message.

Couldn't even find a test for the 0 case (which explains why we failed on it, in retrospect...) so I added a test to cover both in f4e4391

crates/qpy/src/circuit_writer.rs

Cryoris · 2026-03-26T14:06:25Z

crates/qpy/src/circuit_writer.rs

            params.extend(pack_instruction_blocks(instruction, qpy_data)?);
+            match duration {
+                None => {
+                    // we follow the python default


I wonder how helpful all the "Python" comments will be in a few months/years when we come back to this. Especially because now we know we mean V17 but the Python format also changed. Could we maybe put the QPY version and say Python V17 or whichever is the right one? (not just here but there's a bunch of places)

The "Python" part is relevant since it deals with the ad-hoc manner in which the python code itself operates when loading gates, not the QPY17 standard which is vague on this point. If one wants to understand what's going on here, they need to read the Python code (we have version history, it'll survive the coming purge of Python QPY from the codebase).

Let's take an follow-on issue to move all these comments about "Python default" to being documented semantics of QPY in the specification.

Cryoris · 2026-03-26T14:13:25Z

crates/qpy/src/circuit_writer.rs

+                    ))
+                })
+                .collect::<Result<Vec<GenericValue>, _>>()?;
+            let cases = GenericValue::Tuple(


We could just merge this into the case_labels generation and avoid collecting them explicitly, couldn't we?

Yes, that's a refactoring I didn't think through. Fixed in f4e4391

crates/qpy/src/params.rs

jakelishman

I've left a few comments to follow up on, and the rest of this comment here is because I wanted to clarify some points in the main PR comment.

I believe this fixes all the current known issues against QPY. I suspect there might be more lurking, but I think we've probably squashed the main ones now, and it's best to try and get out an rc2.

ParameterVector in qpy_reply

In Python, the data type of a ParameterVector inside the qpy_replay object is stored as Parameter (b'p') and not ParameterVector (b'v'). Rust saved them with separate types, resulting in Python failing to load QPY files generated with Rust when the circuits contained parameter expressions involving ParameterVectors.

I just really want to stress that the fact that there is a key called Parameter with magic sigil p here in the ParameterExpression replay format does not mean that it is in any way related to the same key called Parameter with magic sigil p used in a different place in the QPY format. The naming is confusing, but in Rust terms: think of the available magic sigils in any given place in the spec as completely different enums.

In Rust, the BoxOp duration parameter was stored as GenericData which is either duration or expression. This should be the implementation going forward to QPY18 and beyond, but in Python support for duration in instruction parameters was nonexistant, so a hack was used instead, where BoxOp had two parameters, one for the numerical value and the other for the string of the duration's units - as well as "expr" when storing an expression. The current Rust code now conforms to this hack.

This isn't a hack (or at least, not any more than the rest of QPY loading anything other than a standard gate, barrier or measure is) - it's because there's an undocumented and implied behaviour needed to load QPY that for the "magic" built-in Qiskit objects, you construct them by doing ClassType(*params). Since BoxOp is constructed as BoxOp(body, duration, unit, label, annotations) where duration, unit uses the same float | expr.Expr, str thing as Delay, that's how you end up with the behaviour you observed.

Now the fact that QPY does not specify which objects have to be loaded as MagicClassName(*params) and which as Instruction(name, num_qubits, num_clbits, params) is part of why I say it's not a fully specified format.

Previously, Rust stored the "new style" control flow names, e.g. if_else. This makes Python unable to correctly create those gates as it expects the actual class name.

This is the same problem as the last paragraph above: there are "magic" names that the QPY specification does not mention, but are critical for reloading the same semantics.

jakelishman · 2026-03-26T18:09:25Z

crates/qpy/src/circuit_reader.rs

+                // we follow the python way of storing switch params
+                // the first param is the target, the next param is the cases specifier
+                // the cases specifier is a list of pairs (tuples)
+                // the second element in each pair is the subcircuit for this case
+                // the first element is the list of the case labels, or a single case label
+                // or the special default case label


we follow the python way

For the Rust-space QPY dumper to maintain the same semantics as the Python one (i.e. to be cross-compatible), we must always treat anything special that the Python-space one did with parameters based on the instruction name as actually being part of the QPY spec and simply not written down.

We should also follow-up to write down all these special cases in the specification; you cannot load up a circuit from QPY that contains one of the special-cased objects without knowing what the intended semantics are.

jakelishman · 2026-03-26T18:11:15Z

crates/qpy/src/circuit_reader.rs

+                }
+                (target_value, case_label_list)
+            } else {
+                param_values = instruction_values.split_off(3);


Let's take a follow-on action item to either insert a comment explaining that this path should only be taken by the to-be-yanked 2.4.0rc1, or to delete the code.

jakelishman · 2026-03-26T18:12:28Z

crates/qpy/src/circuit_writer.rs

            params.extend(pack_instruction_blocks(instruction, qpy_data)?);
+            match duration {
+                None => {
+                    // we follow the python default


Let's take an follow-on issue to move all these comments about "Python default" to being documented semantics of QPY in the specification.

jakelishman · 2026-03-26T18:13:14Z

crates/qpy/src/circuit_writer.rs

+            // we follow the python way of storing switch params
+            // the first param is the target, the next param is the cases specificer
+            // the cases specifier is a list of pairs (tuples)
+            // the second element in each pair is the subcircuit for this case
+            // the first element is the list of the case labels, or a single case label
+            // or the special default case label


Another thing that should move to documentation of the specification.

jakelishman · 2026-03-26T18:46:22Z

Oh also, I made the "round trip" tests more aggressive at testing every combination of read/write between Rust and Python space in 84686c8, which gave me a bit more faith in the suite.

coveralls · 2026-03-26T19:18:33Z

Pull Request Test Coverage Report for Build 23611986698

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

106 of 144 (73.61%) changed or added relevant lines in 4 files are covered.
473 unchanged lines in 20 files lost coverage.
Overall coverage increased (+1.1%) to 88.354%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
crates/qpy/src/value.rs	5	11	45.45%
crates/qpy/src/circuit_writer.rs	66	75	88.0%
crates/qpy/src/circuit_reader.rs	33	56	58.93%

Files with Coverage Reduction	New Missed Lines	%
crates/qasm2/src/expr.rs	1	93.82%
crates/transpiler/src/passes/unitary_synthesis/mod.rs	2	96.89%
crates/qasm2/src/lex.rs	3	92.8%
crates/qpy/src/bytes.rs	3	63.4%
crates/cext/src/circuit.rs	4	78.3%
crates/circuit/src/parameter/symbol_expr.rs	4	74.22%
crates/circuit/src/nlayout.rs	6	74.83%
crates/circuit/src/packed_instruction.rs	7	92.34%
crates/transpiler/src/passes/litinski_transformation.rs	8	95.95%
crates/synthesis/src/linalg/mod.rs	10	87.33%

Totals
Change from base Build 23345366698:	1.1%
Covered Lines:	93590
Relevant Lines:	105926

💛 - Coveralls

* Change the encoding of qpy_replay ParameterVector elements to what the Python module expects * Add test * Fix another two bugs found by the extended testing * BoxOp parameter handling * Rust writer now writes SwitchCaseOp's params in the python style * Reading of python style SwitchCaseOp in Rust * Make for_loop tuple params be saved as little endian * Fix BoxOp handling of Nulll duration * Reconcile new and old reading of SwitchCaseOp * Make control_flow_class_name use the enum, not the string * Bugfix in parameter vector lookup for QPY<15 * Raise an error when condition value is too large * Fixes according to PR review * Bugfix * Apply suggestions from code review Co-authored-by: Eli Arbel <46826214+eliarbel@users.noreply.github.com> * Fixes according to PR review * Ensure all combinations of read/write run --------- Co-authored-by: Eli Arbel <46826214+eliarbel@users.noreply.github.com> Co-authored-by: Jake Lishman <jake.lishman@ibm.com> (cherry picked from commit 63ccd7d)

* Change the encoding of qpy_replay ParameterVector elements to what the Python module expects * Add test * Fix another two bugs found by the extended testing * BoxOp parameter handling * Rust writer now writes SwitchCaseOp's params in the python style * Reading of python style SwitchCaseOp in Rust * Make for_loop tuple params be saved as little endian * Fix BoxOp handling of Nulll duration * Reconcile new and old reading of SwitchCaseOp * Make control_flow_class_name use the enum, not the string * Bugfix in parameter vector lookup for QPY<15 * Raise an error when condition value is too large * Fixes according to PR review * Bugfix * Apply suggestions from code review * Fixes according to PR review * Ensure all combinations of read/write run --------- (cherry picked from commit 63ccd7d) Co-authored-by: gadial <gadial@gmail.com> Co-authored-by: Eli Arbel <46826214+eliarbel@users.noreply.github.com> Co-authored-by: Jake Lishman <jake.lishman@ibm.com>

gadial added 2 commits March 22, 2026 12:00

Change the encoding of qpy_replay ParameterVector elements to what th…

4f17ee3

…e Python module expects

Add test

679b1f4

gadial requested a review from a team as a code owner March 22, 2026 10:10

ShellyGarion added the Changelog: Fixed Add a "Fixed" entry in the GitHub Release changelog. label Mar 22, 2026

ShellyGarion added this to the 2.4.0 milestone Mar 22, 2026

github-project-automation bot added this to Qiskit 2.4 Mar 22, 2026

github-project-automation bot moved this to Ready in Qiskit 2.4 Mar 22, 2026

gadial added 2 commits March 22, 2026 15:14

Fix another two bugs found by the extended testing

945b92f

BoxOp parameter handling

04ef62f

Cryoris reviewed Mar 23, 2026

View reviewed changes

gadial added 2 commits March 23, 2026 11:31

Rust writer now writes SwitchCaseOp's params in the python style

b25ed57

Reading of python style SwitchCaseOp in Rust

035141f

gadial changed the title ~~Change the encoding of qpy_replay ParameterVector elements~~ QPY Rust and Python compatibility fixes Mar 23, 2026

gadial marked this pull request as draft March 23, 2026 10:34

gadial added 3 commits March 23, 2026 13:57

Make for_loop tuple params be saved as little endian

4ac6537

Fix BoxOp handling of Nulll duration

04dedc3

Reconcile new and old reading of SwitchCaseOp

2a401f6

gadial mentioned this pull request Mar 23, 2026

QPY dumper in 2.4 does not handle conditions with registers #15856

Closed

mtreinish added the stable backport potential Make Mergify open a backport PR to the most recent stable branch on merge. label Mar 23, 2026

gadial marked this pull request as ready for review March 24, 2026 10:05

gadial added 2 commits March 24, 2026 12:24

Make control_flow_class_name use the enum, not the string

61ed236

Bugfix in parameter vector lookup for QPY<15

8b00d4e

eliarbel reviewed Mar 24, 2026

View reviewed changes

crates/qpy/src/circuit_writer.rs Show resolved Hide resolved

eliarbel reviewed Mar 25, 2026

View reviewed changes

crates/qpy/src/circuit_writer.rs Outdated Show resolved Hide resolved

eliarbel reviewed Mar 25, 2026

View reviewed changes

crates/qpy/src/circuit_writer.rs Outdated Show resolved Hide resolved

eliarbel reviewed Mar 25, 2026

View reviewed changes

crates/qpy/src/circuit_writer.rs Outdated Show resolved Hide resolved

eliarbel reviewed Mar 25, 2026

View reviewed changes

gadial requested review from Cryoris and eliarbel March 25, 2026 16:40

eliarbel reviewed Mar 26, 2026

View reviewed changes

crates/qpy/src/circuit_reader.rs Outdated Show resolved Hide resolved

eliarbel reviewed Mar 26, 2026

View reviewed changes

Apply suggestions from code review

6402d04

Co-authored-by: Eli Arbel <46826214+eliarbel@users.noreply.github.com>

1ucian0 linked an issue Mar 26, 2026 that may be closed by this pull request

QPY dumper in 2.4 does not handle conditions with registers #15856

Closed

jakelishman modified the milestones: 2.4.0, 2.4.0rc2 Mar 26, 2026

Cryoris reviewed Mar 26, 2026

View reviewed changes

Fixes according to PR review

f4e4391

gadial requested a review from Cryoris March 26, 2026 16:21

Ensure all combinations of read/write run

84686c8

jakelishman approved these changes Mar 26, 2026

View reviewed changes

jakelishman enabled auto-merge March 26, 2026 18:45

jakelishman added this pull request to the merge queue Mar 26, 2026

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 26, 2026

jakelishman added this pull request to the merge queue Mar 26, 2026

alexanderivrii mentioned this pull request Mar 26, 2026

Prepare 2.4.0rc2 #15886

Merged

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 26, 2026

Cryoris added this pull request to the merge queue Mar 27, 2026

Merged via the queue into Qiskit:main with commit 63ccd7d Mar 27, 2026
25 checks passed

github-project-automation bot moved this from Ready to Done in Qiskit 2.4 Mar 27, 2026

mergify bot mentioned this pull request Mar 27, 2026

QPY Rust and Python compatibility fixes (backport #15847) #15887

Merged

gadial deleted the qpy_parameter_replay_vector_fix branch March 27, 2026 08:31

Conversation

gadial commented Mar 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Details and comments

ParameterVector in qpy_reply

BoxOp duration parameter

Control flow names

SwitchCaseOp handling

Additional bugfixes

Uh oh!

qiskit-bot commented Mar 22, 2026

Uh oh!

Cryoris left a comment

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!

Uh oh!

Uh oh!

gadial commented Mar 23, 2026

Uh oh!

qiskit-bot commented Mar 24, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eliarbel Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

eliarbel Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Cryoris left a comment

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!

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

gadial commented Mar 22, 2026 •

edited

Loading

eliarbel Mar 26, 2026 •

edited

Loading

eliarbel Mar 26, 2026 •

edited

Loading