Thanks Marc, this issue affects me too.

In trying to find a workaround for this issue, I believe I uncovered another, somewhat related bug in CommutationChecker.

The attempted workaround was to convert the PauliGate to an equivalent PauliEvolutionGate before passing it to the CommutationChecker.

I'm not familiar with where PauliEvolutionGate stores the Pauli definition, but it's not in the list of parameters, and at first it looked like the commutation checker was processing the instruction correctly.

The problem is that two distinct PauliEvolutionGate objects can have seemingly identical Instructions, and the CommutationChecker's caching does not distinguish between them. For example:

pauli_gate = PauliEvolutionGate(qi.Pauli('XX'), time=np.pi/2)
pauli_gate2 = PauliEvolutionGate(qi.Pauli('YY'), time=np.pi/2)

If we print these, we see the instructions appear superficially identical:

Instruction(name='PauliEvolution', num_qubits=2, num_clbits=0, params=[1.5707963267948966])
Instruction(name='PauliEvolution', num_qubits=2, num_clbits=0, params=[1.5707963267948966])

CommutationChecker will correctly say that pauli_gate commutes with an RXGate on qubit 0. But if you subsequently ask it if pauli_gate2 commutes with an RXGate on qubit 0, it will wrongly identify pauli_gate2 with the cached result for pauli_gate and conclude that pauli_gate2 also commutes with an RXGate on qubit 0.

Reproducer:

from qiskit.circuit.library import PauliGate, RXGate, PauliEvolutionGate
from qiskit.circuit import Parameter
from qiskit.circuit.commutation_library import CommutationChecker
import qiskit.quantum_info as qi
import numpy as np

rx_gate_theta = RXGate(Parameter("Theta"))

pauli_gate = PauliEvolutionGate(qi.Pauli('XX'), time=np.pi/2)
pauli_gate2 = PauliEvolutionGate(qi.Pauli('YY'), time=np.pi/2)

cc = CommutationChecker()
print(cc.commute(pauli_gate, [0, 1], [], rx_gate_theta, [0], []))
print(cc.commute(pauli_gate2, [0, 1], [], rx_gate_theta, [0], []))

Output:

True  ## this is correct
True  ## this is not

So when fixing the hashing for PauliGate in this issue, we could also likewise fix how the hashing is done for PauliEvolutionGate.

Thanks for the reports! Indeed for the PauliEvolutionGate we'd need to do a custom caching, since the Paulis to evolve are not stored in the params. However the more sensible solution is likely just to not cache them (we already have some exceptions for this, e.g. for LinearFunction).

For the PauliGate the issue seems to be that the Python version of the commutation checker supported strings as cache, but the Rust version now doesn't anymore. If the performance cost is acceptable we could also just skip the cache here. Otherwise we'd have to support hash on strings to cache the params here.

@aeddins-ibm: you're actually pointing at something worse -- the commutation checker will try caching a gate it does not know and only use what's inside Gate.params as key. So any gate storing information impacting the definition outside of Gate.params can lead to a wrong result.

We should probably update the commutation checker to use a cache whitelist (i.e. only our standard gates, where we know the definition is only affected by params) instead of a blacklist.