Correctly updating global phase when removing gates that are identity…

… up to a global phase (Qiskit#13785)

* correctly updating global phase when removing -I gates from the circuit

* similar fix for specialized rotation gates

* Correctly updating global phase when removing
gates that are equivalent to identity up to a global phase

* also handling UnitaryGates after the unitary gates PR was merged

* applying review suggestions

* removing code duplication

crates/accelerate/src/remove_identity_equiv.rs

@@ -9,7 +9,6 @@
 // Any modifications or derivative works of this code must retain this
 // copyright notice, and modified files need to carry a notice indicating
 // that they have been altered from the originals.
-
 use num_complex::Complex64;
 use num_complex::ComplexFloat;
 use pyo3::prelude::*;
@@ -27,11 +26,13 @@ use qiskit_circuit::packed_instruction::PackedInstruction;
 #[pyfunction]
 #[pyo3(signature=(dag, approx_degree=Some(1.0), target=None))]
 fn remove_identity_equiv(
+    py: Python,
     dag: &mut DAGCircuit,
     approx_degree: Option<f64>,
     target: Option<&Target>,
 ) {
     let mut remove_list: Vec<NodeIndex> = Vec::new();
+    let mut global_phase_update: f64 = 0.;
 
     let get_error_cutoff = |inst: &PackedInstruction| -> f64 {
         match approx_degree {
@@ -75,12 +76,17 @@ fn remove_identity_equiv(
     };
 
     for (op_node, inst) in dag.op_nodes(false) {
-        match inst.op.view() {
+        if inst.is_parameterized() {
+            // Skip parameterized gates
+            continue;
+        }
+        let view = inst.op.view();
+        match view {
             OperationRef::StandardGate(gate) => {
                 let (dim, trace) = match gate {
                     StandardGate::RXGate | StandardGate::RYGate | StandardGate::RZGate => {
                         if let Param::Float(theta) = inst.params_view()[0] {
-                            let trace = (theta / 2.).cos() * 2.;
+                            let trace = Complex64::new((theta / 2.).cos() * 2., 0.);
                             (2., trace)
                         } else {
                             continue;
@@ -91,55 +97,55 @@ fn remove_identity_equiv(
                     | StandardGate::RZZGate
                     | StandardGate::RZXGate => {
                         if let Param::Float(theta) = inst.params_view()[0] {
-                            let trace = (theta / 2.).cos() * 4.;
+                            let trace = Complex64::new((theta / 2.).cos() * 4., 0.);
                             (4., trace)
                         } else {
                             continue;
                         }
                     }
                     _ => {
-                        // Skip global phase gate
-                        if gate.num_qubits() < 1 {
-                            continue;
-                        }
                         if let Some(matrix) = gate.matrix(inst.params_view()) {
                             let dim = matrix.shape()[0] as f64;
-                            let trace = matrix.diag().iter().sum::<Complex64>().abs();
+                            let trace = matrix.diag().iter().sum::<Complex64>();
                             (dim, trace)
                         } else {
                             continue;
                         }
                     }
                 };
                 let error = get_error_cutoff(inst);
-                let f_pro = (trace / dim).powi(2);
+                let f_pro = (trace / dim).abs().powi(2);
                 let gate_fidelity = (dim * f_pro + 1.) / (dim + 1.);
                 if (1. - gate_fidelity).abs() < error {
-                    remove_list.push(op_node)
+                    remove_list.push(op_node);
+                    global_phase_update += (trace / dim).arg();
                 }
             }
-            OperationRef::Gate(gate) => {
-                // Skip global phase like gate
-                if gate.num_qubits() < 1 {
-                    continue;
-                }
-                if let Some(matrix) = gate.matrix(inst.params_view()) {
+            _ => {
+                let matrix = view.matrix(inst.params_view());
+                // If view.matrix() returns None, then there is no matrix and we skip the operation.
+                if let Some(matrix) = matrix {
                     let error = get_error_cutoff(inst);
                     let dim = matrix.shape()[0] as f64;
                     let trace: Complex64 = matrix.diag().iter().sum();
                     let f_pro = (trace / dim).abs().powi(2);
                     let gate_fidelity = (dim * f_pro + 1.) / (dim + 1.);
                     if (1. - gate_fidelity).abs() < error {
-                        remove_list.push(op_node)
+                        remove_list.push(op_node);
+                        global_phase_update += (trace / dim).arg();
                     }
                 }
             }
-            _ => continue,
         }
     }
     for node in remove_list {
         dag.remove_op_node(node);
     }
+
+    if global_phase_update != 0. {
+        dag.add_global_phase(py, &Param::Float(global_phase_update))
+            .expect("The global phase is guaranteed to be a float");
+    }
 }
 
 pub fn remove_identity_equiv_mod(m: &Bound<PyModule>) -> PyResult<()> {

qiskit/transpiler/passes/optimization/remove_identity_equiv.py

@@ -23,9 +23,8 @@
 class RemoveIdentityEquivalent(TransformationPass):
     r"""Remove gates with negligible effects.
 
-    Removes gates whose effect is close to an identity operation, up to the specified
-    tolerance. Zero qubit gates such as :class:`.GlobalPhaseGate` are not considered
-    by this pass.
+    Removes gates whose effect is close to an identity operation up to a global phase
+    and up to the specified tolerance. Parameterized gates are not considered by this pass.
 
     For a cutoff fidelity :math:`f`, this pass removes gates whose average
     gate fidelity with respect to the identity is below :math:`f`. Concretely,

releasenotes/notes/fix-phase-in-remove-id-equivalent-6480da0c62f20df1.yaml

@@ -0,0 +1,8 @@
+---
+fixes:
+  - |
+    Fixed a bug in the :class:`.RemoveIdentityEquivalent` transpiler pass, where gates close
+    to identity up to a global phase were removed from the circuit,
+    but the global phase of the circuit was not updated. In particular,
+    :class:`.RemoveIdentityEquivalent` now removes non-parameterized :class:`.GlobalPhaseGate`
+    gates.

test/python/transpiler/test_remove_identity_equivalent.py

@@ -12,6 +12,7 @@
 
 """Tests for the DropNegligible transpiler pass."""
 
+import ddt
 import numpy as np
 
 from qiskit.circuit import Parameter, QuantumCircuit, QuantumRegister, Gate
@@ -26,6 +27,7 @@
     XXMinusYYGate,
     XXPlusYYGate,
     GlobalPhaseGate,
+    UnitaryGate,
 )
 from qiskit.quantum_info import Operator
 from qiskit.transpiler.passes import RemoveIdentityEquivalent
@@ -34,6 +36,7 @@
 from test import QiskitTestCase  # pylint: disable=wrong-import-order
 
 
+@ddt.ddt
 class TestDropNegligible(QiskitTestCase):
     """Test the DropNegligible pass."""
 
@@ -173,13 +176,33 @@ def to_matrix(self):
         expected = QuantumCircuit(3)
         self.assertEqual(expected, transpiled)
 
-    def test_global_phase_ignored(self):
-        """Test that global phase gate isn't considered."""
+    @ddt.data(
+        RXGate(0),
+        RXGate(2 * np.pi),
+        RYGate(0),
+        RYGate(2 * np.pi),
+        RZGate(0),
+        RZGate(2 * np.pi),
+        UnitaryGate(np.array([[1, 0], [0, 1]])),
+        UnitaryGate(np.array([[-1, 0], [0, -1]])),
+        UnitaryGate(np.array([[np.exp(1j * np.pi / 4), 0], [0, np.exp(1j * np.pi / 4)]])),
+        GlobalPhaseGate(0),
+        GlobalPhaseGate(np.pi / 4),
+    )
+    def test_remove_identity_up_to_global_phase(self, gate):
+        """Test that gates equivalent to identity up to a global phase are removed from the circuit,
+        and the global phase of the circuit is updated correctly.
+        """
+        qc = QuantumCircuit(gate.num_qubits)
+        qc.append(gate, qc.qubits)
+        transpiled = RemoveIdentityEquivalent()(qc)
+        self.assertEqual(transpiled.size(), 0)
+        self.assertEqual(Operator(qc), Operator(transpiled))
 
+    def test_parameterized_global_phase_ignored(self):
+        """Test that parameterized global phase gates are not removed by the pass."""
+        theta = Parameter("theta")
         qc = QuantumCircuit(1)
-        qc.id(0)
-        qc.append(GlobalPhaseGate(0))
+        qc.append(GlobalPhaseGate(theta), [])
         transpiled = RemoveIdentityEquivalent()(qc)
-        expected = QuantumCircuit(1)
-        expected.append(GlobalPhaseGate(0))
-        self.assertEqual(transpiled, expected)
+        self.assertEqual(qc, transpiled)