Skip to content

[WIP] Fix QPY fails expressions with cancelled-out variables #15355 #15397

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
bruno-ah-um wants to merge 2 commits into
base: main
Choose a base branch
from
+33 −3
Draft

[WIP] Fix QPY fails expressions with cancelled-out variables #15355 #15397

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5c3317b
Fix QPY fails to roundtrip expressions with cancelled-out variables (…
bruno-ah-um Nov 29, 2025
2c4dd6f
Merge branch 'main' into qpy-fails-expression-cancelled-out-vars
bruno-ah-um Dec 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 24 additions & 3 deletions crates/circuit/src/parameter/parameter_expression.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,7 @@ impl ParameterExpression {
OpCode::LOG => lhs.log(),
OpCode::EXP => lhs.exp(),
OpCode::SIGN => lhs.sign(),
OpCode::POS => lhs,
OpCode::GRAD | OpCode::SUBSTITUTE => {
panic!("GRAD and SUBSTITUTE are not supported.")
}
Expand Down Expand Up @@ -1922,6 +1923,7 @@ pub enum OpCode {
RSUB = 18,
RDIV = 19,
RPOW = 20,
POS = 21,
}

impl From<OpCode> for u8 {
Expand Down Expand Up @@ -2042,6 +2044,25 @@ pub fn qpy_replay(
expr: &ParameterExpression,
name_map: &HashMap<String, Symbol>,
replay: &mut Vec<OPReplay>,
) {
match &expr.expr {
SymbolExpr::Value(_) | SymbolExpr::Symbol(_) => {
// We generate a POS (identity) operation for Values and Symbols for expressions with all cancelled-out variables
let lhs_value = ParameterValueType::extract_from_expr(&expr.expr);
replay.push(OPReplay {
op: OpCode::POS,
lhs: lhs_value,
rhs: None,
});
}
_ => qpy_replay_helper(expr, name_map, replay),
}
}

fn qpy_replay_helper(
expr: &ParameterExpression,
name_map: &HashMap<String, Symbol>,
replay: &mut Vec<OPReplay>,
Comment on lines +2048 to +2065
Copy link

@debasmita2102 debasmita2102 Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the expression simplification step always guaranteed to turn a “cancelled‑out” expression into a plain Value or Symbol before qpy_replay runs, or is it possible that qpy_replay_helper still receives a more complicated expression tree that, mathematically, is just a constant (for example, several nested adds and multiplies that reduce to zero)?

If that second case can happen, would it be safer for qpy_replay_helper to check whether filter_name_map has removed all parameters from the expression and, in that situation, emit a single POS replay step so that all constant‑after‑cancellation expressions are treated the same way during QPY replay?

Copy link
Author

@bruno-ah-um bruno-ah-um Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The proposed qpy_replay_helper is the old qpy_replay, it didn't change. It is a recursive function. In this case we have special case for the first iteration. The whole issue is that replay generates a list of operations, and the POS operation is just a way of generating a fake operation for representing a single value.

In the case you are describing is not an issue. Consider the following code

x = Parameter("x")
y = Parameter("y")
qc.rz(1 + y + 0 * x, 0)

the code will generate and ADD operation with 1 and y as operands. X will be discarded.

) {
match &expr.expr {
SymbolExpr::Value(_) | SymbolExpr::Symbol(_) => {
Expand All @@ -2066,7 +2087,7 @@ pub fn qpy_replay(
let lhs = filter_name_map(expr, name_map);

// recurse on the instruction
qpy_replay(&lhs, name_map, replay);
qpy_replay_helper(&lhs, name_map, replay);

let lhs_value = ParameterValueType::extract_from_expr(expr);

Expand All @@ -2092,8 +2113,8 @@ pub fn qpy_replay(
// recurse on the parameter expressions
let lhs = filter_name_map(lhs, name_map);
let rhs = filter_name_map(rhs, name_map);
qpy_replay(&lhs, name_map, replay);
qpy_replay(&rhs, name_map, replay);
qpy_replay_helper(&lhs, name_map, replay);
qpy_replay_helper(&rhs, name_map, replay);

// add the expression to the replay
match lhs_value {
Expand Down
1 change: 1 addition & 0 deletions qiskit/circuit/parameterexpression.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
"__rsub__",
"__rtruediv__",
"__rpow__",
"__pos__",
)


Expand Down
8 changes: 8 additions & 0 deletions test/python/qpy/test_circuit_load_from_qpy.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,14 @@ def test_rdiv(self):
qc.ry(expr, 0)
self.assert_roundtrip_equal(qc)

def test_pos(self):
"""Test expressions with cancelled-out variables works as expected"""
qc = QuantumCircuit(1)
a = Parameter("A")
expr = 0 * a
qc.ry(expr, 0)
self.assert_roundtrip_equal(qc)


@ddt
class TestUseSymengineFlag(QpyCircuitTestCase):
Expand Down