CommutationAnalysis needs to be refactored #8020
Description
in 2018, #1500 introduced CommutationAnalysis pass. The pass does the trick while working with CommutativeCancellation. However, there are some aspects that are not very well covered and I think it is time to sit again with it and think it over again. Here there are a list of the problems:
property commutation_set is a messy dict
The property property_set['commutation_set'] is a non-uniformly typed dict:
from qiskit import QuantumCircuit
circuit = QuantumCircuit(1)
circuit.h(0)
from qiskit.transpiler.passes import CommutationAnalysis
property_set = {}
CommutationAnalysis()(circuit, property_set)
for k,v in property_set['commutation_set'].items():
print(type(k), type(v))<class 'qiskit.circuit.quantumregister.Qubit'> <class 'list'>
<class 'tuple'> <class 'int'>
<class 'tuple'> <class 'int'>
<class 'tuple'> <class 'int'>
suggested solution: property_set['commutation_set'] should be a qiskit.dagcircuit.dagdependency.DAGDependency.
does not handle intransitivity
The current commutation analysis assumes transitivity (I think), while the relation is intransitive. For example H commutes with I and I commutes with Z, but H does not commute with Z. This is not reflected in the commutative set:
from qiskit.circuit import Qubit
from qiskit.dagcircuit import DAGOpNode
def pp_commutation_set(property_commutation_set):
# pretty printer of the commutation set
for qbit, sets in property_commutation_set.items():
if not isinstance(qbit, Qubit):
continue
print(qbit.index)
for commutation_set in sets:
r=[]
for node in commutation_set:
if isinstance(node, DAGOpNode):
r.append(f"{node.op.name}_{node._node_id}{[arg.index for arg in node.qargs]}")
if r:
print('\t', r)
from qiskit import QuantumCircuit
circuit = QuantumCircuit(1)
circuit.h(0)
circuit.id(0)
circuit.z(0)
from qiskit.transpiler.passes import CommutationAnalysis
property_set = {}
CommutationAnalysis()(circuit, property_set)
pp_commutation_set(property_set['commutation_set'])0
['h_2[0]', 'id_3[0]', 'z_4[0]']
There is a lot of duplication with DAGDependency
The qiskit.dagcircuit.dagdependency.DAGDependency seems to do the same as qiskit.transpiler.passes.optimization.commutation_analysis.CommutationAnalysis. For example qiskit.dagcircuit.dagdependency._does_commute and qiskit.transpiler.passes.optimization.commutation_analysis._commute are similar in their goal, but one of them has a cache system that makes it better.