fix(gate-decompose): correct the sign on the first RZ of the RX sequence

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,153 @@ 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)]]
+    }
+
+    /// 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)]]
+    }
+
+    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<C> = 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) => {
+                        // 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.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).
+    /// 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");
+
+            // 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() {
+                    "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})"
+            );
+        }
+    }
 }