-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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.
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
add DropNegligible transpiler pass #12384
base: main
Are you sure you want to change the base?
Changes from 1 commit
e442e7a
f358c1e
136c476
9f57938
035eae1
5483856
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,70 @@ | ||||||||||
# This code is part of Qiskit. | ||||||||||
# | ||||||||||
# (C) Copyright IBM 2024. | ||||||||||
# | ||||||||||
# This code is licensed under the Apache License, Version 2.0. You may | ||||||||||
# obtain a copy of this license in the LICENSE.txt file in the root directory | ||||||||||
# of this source tree or at http://www.apache.org/licenses/LICENSE-2.0. | ||||||||||
# | ||||||||||
# 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. | ||||||||||
|
||||||||||
"""Transpiler pass to drop gates with negligible effects.""" | ||||||||||
|
||||||||||
from __future__ import annotations | ||||||||||
|
||||||||||
import numpy as np | ||||||||||
from qiskit.circuit.library import ( | ||||||||||
CPhaseGate, | ||||||||||
PhaseGate, | ||||||||||
RXGate, | ||||||||||
RXXGate, | ||||||||||
RYGate, | ||||||||||
RYYGate, | ||||||||||
RZGate, | ||||||||||
RZZGate, | ||||||||||
XXMinusYYGate, | ||||||||||
XXPlusYYGate, | ||||||||||
) | ||||||||||
from qiskit.dagcircuit import DAGCircuit | ||||||||||
from qiskit.transpiler.basepasses import TransformationPass | ||||||||||
|
||||||||||
# List of Gate classes with the property that if the gate's parameters are all | ||||||||||
# (close to) zero then the gate has (close to) no effect. | ||||||||||
DROP_NEGLIGIBLE_GATE_CLASSES = ( | ||||||||||
CPhaseGate, | ||||||||||
PhaseGate, | ||||||||||
RXGate, | ||||||||||
RYGate, | ||||||||||
RZGate, | ||||||||||
RXXGate, | ||||||||||
RYYGate, | ||||||||||
RZZGate, | ||||||||||
XXPlusYYGate, | ||||||||||
XXMinusYYGate, | ||||||||||
) | ||||||||||
|
||||||||||
|
||||||||||
class DropNegligible(TransformationPass): | ||||||||||
"""Drop gates with negligible effects.""" | ||||||||||
|
||||||||||
def __init__(self, atol: float = 1e-8) -> None: | ||||||||||
"""Initialize the transpiler pass. | ||||||||||
|
||||||||||
Args: | ||||||||||
atol: Absolute numerical tolerance for determining whether a gate's effect | ||||||||||
is negligible. | ||||||||||
""" | ||||||||||
self.atol = atol | ||||||||||
super().__init__() | ||||||||||
|
||||||||||
def run(self, dag: DAGCircuit) -> DAGCircuit: | ||||||||||
for node in dag.op_nodes(): | ||||||||||
if not isinstance(node.op, DROP_NEGLIGIBLE_GATE_CLASSES): | ||||||||||
continue | ||||||||||
if not all(isinstance(param, (int, float, complex)) for param in node.op.params): | ||||||||||
continue | ||||||||||
if np.allclose(node.op.params, 0, atol=self.atol): | ||||||||||
dag.remove_op_node(node) | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm wondering if we want to generalize this for any That being said doing this will obviously be slower though. If we're concerned about runtime performance I'd probably suggest moving this away from using
Suggested change
because I expect the numpy overhead of converting params to an array and all the checking involved with that will be slower that just checking each value in params manually, especially as in general there won't be very many parameters for a gate. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good idea to use the built-ins instead of Numpy, done. I don't think we should try to construct a gate's matrix, due to the performance overhead, and also because it's not feasible for all gates (cost is exponential in the number of qubits). However, I do think we should let the user pass additional gate classes to check. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've added an argument for passing additional gate types. |
||||||||||
return dag |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,112 @@ | ||
# This code is part of Qiskit. | ||
# | ||
# (C) Copyright IBM 2024. | ||
# | ||
# This code is licensed under the Apache License, Version 2.0. You may | ||
# obtain a copy of this license in the LICENSE.txt file in the root directory | ||
# of this source tree or at http://www.apache.org/licenses/LICENSE-2.0. | ||
# | ||
# 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. | ||
|
||
"""Tests for the DropNegligible transpiler pass.""" | ||
|
||
import numpy as np | ||
|
||
from qiskit import QuantumCircuit, QuantumRegister | ||
from qiskit.circuit import Parameter, QuantumCircuit, QuantumRegister | ||
from qiskit.circuit.library import ( | ||
CPhaseGate, | ||
RXGate, | ||
RXXGate, | ||
RYGate, | ||
RYYGate, | ||
RZGate, | ||
RZZGate, | ||
XXMinusYYGate, | ||
XXPlusYYGate, | ||
) | ||
from qiskit.quantum_info import Operator | ||
from qiskit.transpiler import PassManager | ||
from qiskit.transpiler.passes import DropNegligible | ||
|
||
from test import QiskitTestCase # pylint: disable=wrong-import-order | ||
|
||
|
||
class TestDropNegligible(QiskitTestCase): | ||
"""Test the DropNegligible pass.""" | ||
|
||
def test_drops_negligible_gates(self): | ||
"""Test that negligible gates are dropped.""" | ||
qubits = QuantumRegister(2) | ||
circuit = QuantumCircuit(qubits) | ||
a, b = qubits | ||
circuit.append(CPhaseGate(1e-5), [a, b]) | ||
circuit.append(CPhaseGate(1e-8), [a, b]) | ||
circuit.append(RXGate(1e-5), [a]) | ||
circuit.append(RXGate(1e-8), [a]) | ||
circuit.append(RYGate(1e-5), [a]) | ||
circuit.append(RYGate(1e-8), [a]) | ||
circuit.append(RZGate(1e-5), [a]) | ||
circuit.append(RZGate(1e-8), [a]) | ||
circuit.append(RXXGate(1e-5), [a, b]) | ||
circuit.append(RXXGate(1e-8), [a, b]) | ||
circuit.append(RYYGate(1e-5), [a, b]) | ||
circuit.append(RYYGate(1e-8), [a, b]) | ||
circuit.append(RZZGate(1e-5), [a, b]) | ||
circuit.append(RZZGate(1e-8), [a, b]) | ||
circuit.append(XXPlusYYGate(1e-5, 1e-8), [a, b]) | ||
circuit.append(XXPlusYYGate(1e-8, 1e-8), [a, b]) | ||
circuit.append(XXMinusYYGate(1e-5, 1e-8), [a, b]) | ||
circuit.append(XXMinusYYGate(1e-8, 1e-8), [a, b]) | ||
pass_manager = PassManager([DropNegligible()]) | ||
transpiled = pass_manager.run(circuit) | ||
kevinsung marked this conversation as resolved.
Show resolved
Hide resolved
|
||
self.assertEqual(circuit.count_ops()["cp"], 2) | ||
self.assertEqual(transpiled.count_ops()["cp"], 1) | ||
self.assertEqual(circuit.count_ops()["rx"], 2) | ||
self.assertEqual(transpiled.count_ops()["rx"], 1) | ||
self.assertEqual(circuit.count_ops()["ry"], 2) | ||
self.assertEqual(transpiled.count_ops()["ry"], 1) | ||
self.assertEqual(circuit.count_ops()["rz"], 2) | ||
self.assertEqual(transpiled.count_ops()["rz"], 1) | ||
self.assertEqual(circuit.count_ops()["rxx"], 2) | ||
self.assertEqual(transpiled.count_ops()["rxx"], 1) | ||
self.assertEqual(circuit.count_ops()["ryy"], 2) | ||
self.assertEqual(transpiled.count_ops()["ryy"], 1) | ||
self.assertEqual(circuit.count_ops()["rzz"], 2) | ||
self.assertEqual(transpiled.count_ops()["rzz"], 1) | ||
self.assertEqual(circuit.count_ops()["xx_plus_yy"], 2) | ||
self.assertEqual(transpiled.count_ops()["xx_plus_yy"], 1) | ||
self.assertEqual(circuit.count_ops()["xx_minus_yy"], 2) | ||
self.assertEqual(transpiled.count_ops()["xx_minus_yy"], 1) | ||
np.testing.assert_allclose( | ||
np.array(Operator(circuit)), np.array(Operator(transpiled)), atol=1e-7 | ||
) | ||
|
||
def test_handles_parameters(self): | ||
"""Test that gates with parameters are ignored gracefully.""" | ||
qubits = QuantumRegister(2) | ||
circuit = QuantumCircuit(qubits) | ||
a, b = qubits | ||
theta = Parameter("theta") | ||
circuit.append(CPhaseGate(theta), [a, b]) | ||
circuit.append(CPhaseGate(1e-5), [a, b]) | ||
circuit.append(CPhaseGate(1e-8), [a, b]) | ||
pass_manager = PassManager([DropNegligible()]) | ||
transpiled = pass_manager.run(circuit) | ||
self.assertEqual(circuit.count_ops()["cp"], 3) | ||
self.assertEqual(transpiled.count_ops()["cp"], 2) | ||
|
||
def test_handles_number_types(self): | ||
"""Test that gates with different types of numbers are handled correctly.""" | ||
qubits = QuantumRegister(2) | ||
circuit = QuantumCircuit(qubits) | ||
a, b = qubits | ||
circuit.append(CPhaseGate(np.float32(1e-6)), [a, b]) | ||
circuit.append(CPhaseGate(1e-3), [a, b]) | ||
circuit.append(CPhaseGate(1e-8), [a, b]) | ||
pass_manager = PassManager([DropNegligible(atol=1e-5)]) | ||
transpiled = pass_manager.run(circuit) | ||
self.assertEqual(circuit.count_ops()["cp"], 3) | ||
self.assertEqual(transpiled.count_ops()["cp"], 1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since you're using a fixed set of gates in this PR one thing you should do here is before looping is check if the names of the gate classes intersect with
dag.count_ops()
. That'll be much faster in the negative case.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't
dag.count_ops
also need to iterate through the nodes of the dag?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DAGCircuit
maintains a dictionary of counts that it updates whenever an op is added or removed from the circuit. This is explicitly to support cases like this to do constant time checks for the instructions presence in the circuit. The only exceptions are if you setrecurse=True
(which is the default) and there are any control flow ops in the circuit.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dag.count_ops()
stores strings, but I'm using the gate class itself to check the gate type. Any advice? Should I also just use strings? That doesn't seem robust because any gate can have any arbitrary name. But I guess there's many places in Qiskit which assume to know the type of a gate based on its name.