Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
33 changes: 26 additions & 7 deletions crates/transpiler/src/passes/inverse_cancellation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,12 +174,17 @@ static SELF_INVERSE_GATES_FOR_CANCELLATION: [StandardGate; 15] = [
StandardGate::C3X,
];

// Inverse cancellation pairs, plus a static array indicating
// whether the gates are symmetric and the qubit order does not matter.
// For CS-CSdg, for example, the qubit order is irrelevant since the
// gates are symmetric.
static INVERSE_PAIRS_FOR_CANCELLATION: [[StandardGate; 2]; 4] = [
[StandardGate::T, StandardGate::Tdg],
[StandardGate::S, StandardGate::Sdg],
[StandardGate::SX, StandardGate::SXdg],
[StandardGate::CS, StandardGate::CSdg],
];
static INVERSE_PAIRS_SYMMETRIC: [bool; 4] = [false, false, false, true];
Comment thread
Cryoris marked this conversation as resolved.
Outdated

fn std_self_inverse(dag: &mut DAGCircuit) {
if !SELF_INVERSE_GATES_FOR_CANCELLATION
Expand Down Expand Up @@ -242,15 +247,15 @@ fn std_inverse_pairs(dag: &mut DAGCircuit) {
return;
}
// Handle inverse pairs
for [gate_0, gate_1] in INVERSE_PAIRS_FOR_CANCELLATION {
for (pair_idx, [gate_0, gate_1]) in INVERSE_PAIRS_FOR_CANCELLATION.iter().enumerate() {
Comment thread
Cryoris marked this conversation as resolved.
Outdated
if !dag.get_op_counts().contains_key(gate_0.name())
|| !dag.get_op_counts().contains_key(gate_1.name())
{
continue;
}
let filter = |inst: &PackedInstruction| -> bool {
match inst.op.view() {
OperationRef::StandardGate(gate) => gate == gate_0 || gate == gate_1,
OperationRef::StandardGate(gate) => gate == *gate_0 || gate == *gate_1,
_ => false,
}
};
Expand All @@ -264,11 +269,25 @@ fn std_inverse_pairs(dag: &mut DAGCircuit) {
let NodeType::Operation(next_inst) = &dag[nodes[i + 1]] else {
unreachable!("Not an op node");
};
if inst.qubits == next_inst.qubits
&& (inst.op.try_standard_gate() == Some(gate_0)
&& next_inst.op.try_standard_gate() == Some(gate_1))
|| (inst.op.try_standard_gate() == Some(gate_1)
&& next_inst.op.try_standard_gate() == Some(gate_0))

// For symmetric gates, check if the qubits match irrespective of the order
let qubits_match = if INVERSE_PAIRS_SYMMETRIC[pair_idx] {
let inst_qubits = dag.get_qargs(inst.qubits);
let next_inst_qubits = dag.get_qargs(next_inst.qubits);

// We know that we have only 2-qubit gates here, so we perform a brief
// check based on the slice, without converting to a set. We also do not
// have to check the size matches, since we later check that the gates match,
// hence we know the number of qubits matches.
inst_qubits.iter().all(|q| next_inst_qubits.contains(q))
} else {
inst.qubits == next_inst.qubits
};
if qubits_match
&& ((inst.op.try_standard_gate() == Some(*gate_0)
&& next_inst.op.try_standard_gate() == Some(*gate_1))
|| (inst.op.try_standard_gate() == Some(*gate_1)
&& next_inst.op.try_standard_gate() == Some(*gate_0)))
{
dag.remove_op_node(nodes[i]);
dag.remove_op_node(nodes[i + 1]);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
fixes:
- |
Fixed a bug in :class:`.InverseCancellation` where pairs of symmetric gates
(such as :class:`.CSGate` and :class:`.CSdgGate`) were not correctly cancelled
if their qubit arguments were the same but in different order. Symmetric
gates should get cancelled irrespective of the qubit order.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If you write the bugfix note like this, we need to handle the cz case Sasha mentioned too.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

From reading the release note, it seems like we are asserting that InverseCancellation should cancel all symmetric pairs of gates, which it does not.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Lol, this is the second time I start writing a comment and see that Jake already beat me to it.

Fixed `#15855 <https://github.com/Qiskit/qiskit/issues/15855>`__.
32 changes: 32 additions & 0 deletions test/python/transpiler/test_inverse_cancellation.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@
TdgGate,
CZGate,
RZGate,
CSGate,
CSdgGate,
)
from test import QiskitTestCase

Expand Down Expand Up @@ -740,6 +742,36 @@ def test_nested_control_flow(self, gates_to_cancel):

self.assertEqual(pass_(test), expected)

def test_symmetries(self):
"""Test cancellation of symmetric gates.

CS/CSdg should cancel if they are on the same set of qubits, irrespective of the order.
"""
gates = [CSGate(), CSdgGate()]

# We're testing CS and CSdg in every possible combination and qubit order.
# All should cancel.
for i in [0, 1]:
for j in [0, 1]:
for gate_idx in [0, 1]:
with self.subTest(i=i, j=j, gate_idx=gate_idx):
qc = QuantumCircuit(2)
qc.append(gates[gate_idx], [i, (i + 1) % 2])
qc.append(gates[(gate_idx + 1) % 2], [j, (j + 1) % 2])

tqc = InverseCancellation()(qc)
self.assertEqual(tqc.count_ops(), {})
Comment thread
Cryoris marked this conversation as resolved.
Outdated

# sanity check: verify that CS-CSdg doesn't globally get cancelled
with self.subTest(msg="sanity check"):
qc = QuantumCircuit(3)
qc.cs(0, 1)
qc.csdg(2, 1)

tqc = InverseCancellation()(qc)
self.assertEqual(tqc.count_ops().get("cs", 0), 1)
self.assertEqual(tqc.count_ops().get("csdg", 0), 1)


if __name__ == "__main__":
unittest.main()
Loading