Skip to content
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

CommutationChecker fails with large gates. #13828

Open
MarcDrudis opened this issue Feb 12, 2025 · 4 comments
Open

CommutationChecker fails with large gates. #13828

MarcDrudis opened this issue Feb 12, 2025 · 4 comments
Labels
bug Something isn't working

Comments

@MarcDrudis
Copy link
Contributor

Environment

  • Qiskit version: 2.0.0 and 1.3.2
  • Python version: 3.12.8
  • Operating system: Linux(Fedora)

What is happening?

Commutation Checker fails when checking the commutativity of a large PauliGate. It has to do with the fact that it tries to construct its tensor representation and for some reason runs out of indices to iterate over (I am not really sure what is causing that). This is specially sad because in most cases checking the commutativity of a Pauli String should be trivial regardless of its size.

How can we reproduce the issue?

from qiskit.circuit import QuantumCircuit
from qiskit.circuit.commutation_library import SessionCommutationChecker as scc
from qiskit.converters import circuit_to_dag

circuit = QuantumCircuit(9)
circuit.pauli("ZZZZZZZZZ", range(9))
circuit.cx(5, 6)

dag = circuit_to_dag(circuit)
nodes = list(dag.topological_op_nodes())
print(scc.commute_nodes(nodes[0], nodes[1], max_num_qubits=10))

What should happen?

And I get the following error:

thread '<unnamed>' panicked at crates/accelerate/src/unitary_compose.rs:114:67:
index out of bounds: the len is 26 but the index is 26
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Traceback (most recent call last):
  File "/home/marc/Documents/QiskitPRs/issue.py", line 11, in <module>
    print(scc.commute_nodes(nodes[0], nodes[1], max_num_qubits=10))
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/marc/pyenvs/windsurfer/lib64/python3.12/site-packages/qiskit/circuit/commutation_checker.py", line 45, in commute_nodes
    return self.cc.commute_nodes(op1, op2, max_num_qubits)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
pyo3_runtime.PanicException: index out of bounds: the len is 26 but the index is 26

Any suggestions?

I suggest that the commutativity checker becomes Pauli based. By that I mean that most gates diagonalize in the same basis as a Pauli, so we should check the commutativity of the associated Pauli which is a very cheap operation when done efficiently (bitwise). For instance, one can associate a CNOT gate with a ZX Pauli or a S gate with a Z Pauli. Some other cases are less trivial, like for the H gate, where we need to decompose it into X+Z and checking the commutativity is slightly more involved. However, it should still be a much faster operation than instantiating the matrix of the operation, and if we speed up the Pauli commutativity would probably be as fast as a lookup table.

@MarcDrudis MarcDrudis added the bug Something isn't working label Feb 12, 2025
MarcDrudis added a commit to MarcDrudis/qiskit-terra that referenced this issue Feb 18, 2025
@Cryoris
Copy link
Contributor

Cryoris commented Feb 20, 2025

Maybe @sbrandhsn has an idea for what's going on in the einsum matrix multiplication 🙂 Using Pauli-based checks instead is a bit out of scope for this fix though 😉

@sbrandhsn
Copy link
Contributor

The library ndarray_einsum_beta was used to perform the einsum computations. Apparently that library was limited to 26 indices when the code was developed and the original developer ( me ;-) ) apparently forgot to add code for gracefully exiting in that scenario. I don't see an easy way of fixing this without changing the einsum library or handling this special case separately.

@Cryoris
Copy link
Contributor

Cryoris commented Feb 24, 2025

Shouldn't the considered commutation only use 9 indices though, since it checks Z^{\otimes 9} on qubits 0 to 8 with CX on qubits 5 and 6? Or how are the indices counted? 🤔

@jakelishman
Copy link
Member

Julien: you have indices for both the input and output spaces, and you have two operators, so if there's not much overlap of the spaces, you'll get towards having 4x the number of qubits in indices.

github-merge-queue bot pushed a commit that referenced this issue Mar 4, 2025
* First commit with body of the class

* Optimized some features, added warnings and unittests

* commchecker fixed

* Fixed Pauli Indexing (proper unittests are still missing)

* Apply suggestions from code review

Co-authored-by: Julien Gacon <gaconju@gmail.com>

* d

* added get_final_ops

* Modified LightCone inputs and added tests

* Added tests for errors, modified to

* merging

* fixed commutation_checker version

* Sparse Observable tests for LightCone pass

* Fixed typehints

* Added release notes and max_num qubits for LightCone Pass

* Commented test issue #13828

* Commented decorator

* Fixed linting

* bug in reno

* Fixed non-existing cnot gate in releasenotes

* Apply suggestions from code review

Applied suggestions

Co-authored-by: Julien Gacon <gaconju@gmail.com>

* Changed registers and __init__ in unittest

* Changed warning and simplified if logic

* Fixed linting

* Use SessionCommutatorChecker + typos

---------

Co-authored-by: Samuele Piccinelli <Samuele.Piccinelli@ibm.com>
Co-authored-by: Julien Gacon <gaconju@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants