From 12b921f9b20758c98088d8b14e145d317487a708 Mon Sep 17 00:00:00 2001 From: Cleiton Augusto Date: Mon, 17 Aug 2026 19:48:12 -0400 Subject: [PATCH 1/2] fix(gate-decompose): correct the sign on the first RZ of the RX sequence The RX arm emitted rz(-pi/2), sx, rz(pi + theta), sx, rz(pi/2), which does not implement RX(theta) for any angle. The clearest case is theta = 0: the identity compiled to diag(1, -1), which is Z. Changing the first rotation to rz(+pi/2) makes the sequence equal RX(theta) up to global phase. Adds test_rx_decomposition_matches_rx, which builds the operator from whatever the table returns and compares it against RX(theta) at eight angles, so it keeps checking the real entry rather than a copy. The test fails on the old sign and passes on the new one. --- crates/lift-opt/src/gate_decompose.rs | 119 +++++++++++++++++++++++++- 1 file changed, 116 insertions(+), 3 deletions(-) diff --git a/crates/lift-opt/src/gate_decompose.rs b/crates/lift-opt/src/gate_decompose.rs index 717b943..bedc938 100644 --- a/crates/lift-opt/src/gate_decompose.rs +++ b/crates/lift-opt/src/gate_decompose.rs @@ -226,13 +226,13 @@ fn decompose( vec![("angle", Attribute::Float(-std::f64::consts::FRAC_PI_2))], ), ], - // ── RX(theta) -> RZ(-pi/2) SX RZ(pi/2 + theta) RZ(-pi/2) SX RZ(-pi/2) - // Simplified: RX(theta) = RZ(-pi/2) SX RZ(pi + theta) SX RZ(pi/2) + // ── RX(theta) = RZ(pi/2) SX RZ(pi + theta) SX RZ(pi/2), in circuit order. + // Equality holds up to global phase, see test_rx_decomposition_matches_rx. "quantum.rx" => vec![ ( "quantum.rz".into(), vec![0], - vec![("angle", Attribute::Float(-std::f64::consts::FRAC_PI_2))], + vec![("angle", Attribute::Float(std::f64::consts::FRAC_PI_2))], ), ("quantum.sx".into(), vec![0], vec![]), ( @@ -346,4 +346,117 @@ mod tests { // native for IBM, so this should be Unchanged on second run). let _ = (result, rz_before); } + + // ── 2x2 complex helpers, local so this test adds no dependency ── + + type C = (f64, f64); + type M = [[C; 2]; 2]; + + fn cmul(a: C, b: C) -> C { + (a.0 * b.0 - a.1 * b.1, a.0 * b.1 + a.1 * b.0) + } + + fn mmul(a: M, b: M) -> M { + let mut r = [[(0.0, 0.0); 2]; 2]; + for i in 0..2 { + for j in 0..2 { + let x = cmul(a[i][0], b[0][j]); + let y = cmul(a[i][1], b[1][j]); + r[i][j] = (x.0 + y.0, x.1 + y.1); + } + } + r + } + + fn rz_matrix(t: f64) -> M { + let (c, s) = ((t / 2.0).cos(), (t / 2.0).sin()); + [ + [(c, -s), (0.0, 0.0)], + [(0.0, 0.0), (c, s)], + ] + } + + fn sx_matrix() -> M { + [ + [(0.5, 0.5), (0.5, -0.5)], + [(0.5, -0.5), (0.5, 0.5)], + ] + } + + fn rx_matrix(t: f64) -> M { + let (c, s) = ((t / 2.0).cos(), (t / 2.0).sin()); + [ + [(c, 0.0), (0.0, -s)], + [(0.0, -s), (c, 0.0)], + ] + } + + /// True when `a` and `b` describe the same operator up to a global phase. + fn same_up_to_global_phase(a: M, b: M) -> bool { + let mut phase: Option = None; + for i in 0..2 { + for j in 0..2 { + let (ar, ai) = a[i][j]; + let (br, bi) = b[i][j]; + if ar.hypot(ai) < 1e-12 { + if br.hypot(bi) > 1e-9 { + return false; + } + continue; + } + let d = ar * ar + ai * ai; + let ratio = ((br * ar + bi * ai) / d, (bi * ar - br * ai) / d); + match phase { + None => phase = Some(ratio), + Some(p) => { + if (p.0 - ratio.0).abs() > 1e-9 || (p.1 - ratio.1).abs() > 1e-9 { + return false; + } + } + } + } + } + phase.map_or(false, |p| (p.0.hypot(p.1) - 1.0).abs() < 1e-9) + } + + /// The RX entry in the decomposition table must implement RX(theta). + /// This builds the operator from whatever the table returns, so it keeps + /// checking the real entry rather than a copy of it. + #[test] + fn test_rx_decomposition_matches_rx() { + use std::f64::consts::{FRAC_PI_2, PI}; + + for theta in [0.0, 0.3, 1.0, FRAC_PI_2, PI, 2.2, -0.7, 3.9] { + let mut attrs = lift_core::attributes::Attributes::new(); + attrs.set("angle", Attribute::Float(theta)); + let sequence = + decompose("quantum.rx", &attrs).expect("rx should have a decomposition"); + + // The sequence is in circuit order, so each gate multiplies on the left. + let mut built: M = [[(1.0, 0.0), (0.0, 0.0)], [(0.0, 0.0), (1.0, 0.0)]]; + for (name, _qubits, params) in &sequence { + let gate = match name.as_str() { + "quantum.rz" => { + let angle = params + .iter() + .find(|(k, _)| *k == "angle") + .and_then(|(_, v)| match v { + Attribute::Float(f) => Some(*f), + _ => None, + }) + .expect("rz in the table should carry a float angle"); + rz_matrix(angle) + } + "quantum.sx" => sx_matrix(), + other => panic!("unexpected gate {other} in the rx decomposition"), + }; + built = mmul(gate, built); + } + + assert!( + same_up_to_global_phase(built, rx_matrix(theta)), + "rx decomposition does not implement RX({theta})" + ); + } + } } From 400fe69297887759cc3f640b9b7e609a945c5a75 Mon Sep 17 00:00:00 2001 From: Cleiton Augusto Date: Tue, 18 Aug 2026 09:00:37 -0400 Subject: [PATCH 2/2] fix(gate-decompose): satisfy fmt and clippy, and pin the composition order The first version of this branch failed both CI gates: rustfmt reformatted the matrix literals, and clippy rejected map_or under -D warnings. Both are fixed here, and I ran fmt, clippy and the workspace suite locally this time. The ordering comment in the rx test was also claiming something the test could not check. Every entry in the decomposition table is a single gate, a palindrome or a conjugation, so the product is the same in either order and no table entry can pin the convention down. test_composition_is_in_circuit_order does it directly, with an asymmetric pair and an expected matrix written out by hand rather than composed. That second part matters: comparing mmul against mmul passes even when mmul itself is inverted, since the error cancels on both sides. With the literal matrix the test fails when the multiplication order is flipped. Also loosens the phase comparison to be relative to the entry size, since an absolute bound fails on correct code when a matrix entry is near zero, and names the sx convention the test assumes. --- crates/lift-opt/src/gate_decompose.rs | 70 ++++++++++++++++++++------- 1 file changed, 53 insertions(+), 17 deletions(-) diff --git a/crates/lift-opt/src/gate_decompose.rs b/crates/lift-opt/src/gate_decompose.rs index bedc938..218306b 100644 --- a/crates/lift-opt/src/gate_decompose.rs +++ b/crates/lift-opt/src/gate_decompose.rs @@ -370,25 +370,19 @@ mod tests { fn rz_matrix(t: f64) -> M { let (c, s) = ((t / 2.0).cos(), (t / 2.0).sin()); - [ - [(c, -s), (0.0, 0.0)], - [(0.0, 0.0), (c, s)], - ] + [[(c, -s), (0.0, 0.0)], [(0.0, 0.0), (c, s)]] } + /// sqrt(X), which is the OpenQASM stdgates convention and the one + /// qasm_export.rs emits as `sx q[n];`. With the dagger convention instead, + /// the same sequence would implement RX(-theta). fn sx_matrix() -> M { - [ - [(0.5, 0.5), (0.5, -0.5)], - [(0.5, -0.5), (0.5, 0.5)], - ] + [[(0.5, 0.5), (0.5, -0.5)], [(0.5, -0.5), (0.5, 0.5)]] } fn rx_matrix(t: f64) -> M { let (c, s) = ((t / 2.0).cos(), (t / 2.0).sin()); - [ - [(c, 0.0), (0.0, -s)], - [(0.0, -s), (c, 0.0)], - ] + [[(c, 0.0), (0.0, -s)], [(0.0, -s), (c, 0.0)]] } /// True when `a` and `b` describe the same operator up to a global phase. @@ -409,14 +403,55 @@ mod tests { match phase { None => phase = Some(ratio), Some(p) => { - if (p.0 - ratio.0).abs() > 1e-9 || (p.1 - ratio.1).abs() > 1e-9 { + // Compare relative to the entry size. An absolute bound + // here fails on correct code when |a| is near zero, + // since the float noise is divided by a tiny number. + let scale = ar.hypot(ai).max(1.0); + if (p.0 - ratio.0).abs() > 1e-9 * scale + || (p.1 - ratio.1).abs() > 1e-9 * scale + { return false; } } } } } - phase.map_or(false, |p| (p.0.hypot(p.1) - 1.0).abs() < 1e-9) + phase.is_some_and(|p| (p.0.hypot(p.1) - 1.0).abs() < 1e-9) + } + + /// `mmul(gate, built)` has to mean "gate runs after everything in built", + /// which is what reading a decomposition in circuit order requires. + /// + /// This needs its own test: every entry in the table today is a single + /// gate, a palindrome, or a conjugation, and all three give the same + /// product in either order, so no table entry can pin this down. + #[test] + fn test_composition_is_in_circuit_order() { + use std::f64::consts::FRAC_PI_2; + + let x: M = [[(0.0, 0.0), (1.0, 0.0)], [(1.0, 0.0), (0.0, 0.0)]]; + let id: M = [[(1.0, 0.0), (0.0, 0.0)], [(0.0, 0.0), (1.0, 0.0)]]; + + // RZ(pi/2) first, then X. + let mut built = id; + for gate in [rz_matrix(FRAC_PI_2), x] { + built = mmul(gate, built); + } + + // The expected product is written out rather than composed, so that a + // wrong multiplication order cannot cancel itself out on both sides. + let r = std::f64::consts::FRAC_1_SQRT_2; + let expected: M = [[(0.0, 0.0), (r, r)], [(r, -r), (0.0, 0.0)]]; + let reversed: M = [[(0.0, 0.0), (r, -r)], [(r, r), (0.0, 0.0)]]; + + assert!( + same_up_to_global_phase(built, expected), + "composition should apply the first listed gate first" + ); + assert!( + !same_up_to_global_phase(expected, reversed), + "the two orders must be distinguishable, otherwise this test proves nothing" + ); } /// The RX entry in the decomposition table must implement RX(theta). @@ -429,10 +464,11 @@ mod tests { for theta in [0.0, 0.3, 1.0, FRAC_PI_2, PI, 2.2, -0.7, 3.9] { let mut attrs = lift_core::attributes::Attributes::new(); attrs.set("angle", Attribute::Float(theta)); - let sequence = - decompose("quantum.rx", &attrs).expect("rx should have a decomposition"); + let sequence = decompose("quantum.rx", &attrs).expect("rx should have a decomposition"); - // The sequence is in circuit order, so each gate multiplies on the left. + // Circuit order, so each gate multiplies on the left. That + // convention is pinned by test_composition_is_in_circuit_order, + // since the rx sequence is a palindrome and cannot pin it here. let mut built: M = [[(1.0, 0.0), (0.0, 0.0)], [(0.0, 0.0), (1.0, 0.0)]]; for (name, _qubits, params) in &sequence { let gate = match name.as_str() {