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

commutation_checker.py tries to create identity operator with size 2^(2^num_qubits) #9197

Closed
mstnb opened this issue Nov 24, 2022 · 5 comments · Fixed by #9201
Closed

commutation_checker.py tries to create identity operator with size 2^(2^num_qubits) #9197

mstnb opened this issue Nov 24, 2022 · 5 comments · Fixed by #9201
Assignees
Labels
bug Something isn't working

Comments

@mstnb
Copy link
Contributor

mstnb commented Nov 24, 2022

Environment

  • Qiskit Terra version: 0.22.2
  • Python version: 3.10.8
  • Operating system: Manjaro Linux 22.0.0

What is happening?

I am trying to transpile a deep 10 qubit circuit with optimization_level=3. This results in

ValueError: Maximum allowed dimension exceeded" in qiskit/circuit/commutation_checker.py, line 27, in _identity_op

How can we reproduce the issue?

I broke my code down to this simple example:

import numpy as np
from qiskit import Aer, transpile, QuantumCircuit
from qiskit.circuit.library import XGate

n_qubits = 4
circ = QuantumCircuit(n_qubits + 1)
ctrl_str = format(np.random.randint(0, 2**n_qubits), "b").zfill(n_qubits)
mcx_gate =  XGate().control(num_ctrl_qubits=n_qubits, ctrl_state=ctrl_str).repeat(2)
circ.append(mcx_gate, list(range(n_qubits + 1)))
transpile(circ, backend=Aer.get_backend("aer_simulator"), optimization_level=3)

This fails with

numpy.core._exceptions._ArrayMemoryError: Unable to allocate 32.0 GiB for an array with shape (65536, 65536) and data type float64

on my system with 16GiB of memory.

What should happen?

Transpilation should proceed without any issues.

Any suggestions?

Simple fix: _identity_op() in commutation_checker.py takes as argument num_qubits:
https://github.com/Qiskit/qiskit-terra/blob/e775b4d19ad62558f30f39814b3d923ce03c8654/qiskit/circuit/commutation_checker.py#L24-L28
but is called with 2**num_qubits instead:
https://github.com/Qiskit/qiskit-terra/blob/e775b4d19ad62558f30f39814b3d923ce03c8654/qiskit/circuit/commutation_checker.py#L145-L147
So we simply need to replace the argument in line 147 with extra_qarg2. I don't know how to fix an issue unfortunately.

@mstnb mstnb added the bug Something isn't working label Nov 24, 2022
@jakelishman
Copy link
Member

Thanks - your solution looks exactly correct to me. Would you like to try fixing it? If so, we have a general contributing-to-Qiskit guide and a supplement specific to Terra that have instructions on how to do it, and you can feel free to ask here as well.

If not, no worries - just let us know here and we (or somebody else from the community) will take care of it.

@mstnb
Copy link
Contributor Author

mstnb commented Nov 24, 2022

Great, thanks for your quick response! If it's not too complicated I would like to try, might be a good way to learn more about git.

@jakelishman
Copy link
Member

Sounds good to me - feel free to ask questions here if you need to, and either I or one of the community team will help out. Don't worry if you get things wrong or forget something - there's nothing you can do to actually damage things on this repo.

@jakelishman
Copy link
Member

I've assigned you to the issue so others know somebody's working on it, but don't worry if you need to drop out or anything - just let us know and I'll remove it again. There's no time pressure or anything, it's just a tracker.

@mstnb
Copy link
Contributor Author

mstnb commented Nov 24, 2022

Thanks, I hope everything is fine so far!

@mergify mergify bot closed this as completed in #9201 Mar 1, 2023
mergify bot added a commit that referenced this issue Mar 1, 2023
* Fix wrong argument supplied to _identity_op() #9197

* Add test case with large qubit gate

This commit adds a test case to ensure we get the correct result with a
large number of qubits. This also implicitly tests that we've fixed the
excessive memory consumption because the memory requirements for an 8
qubit gate with the bug still present would not be runnable on most
current systems.

Co-authored-by: Jake Lishman <[email protected]>

* Add release note

---------

Co-authored-by: Matthew Treinish <[email protected]>
Co-authored-by: Jake Lishman <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
mergify bot pushed a commit that referenced this issue Mar 1, 2023
* Fix wrong argument supplied to _identity_op() #9197

* Add test case with large qubit gate

This commit adds a test case to ensure we get the correct result with a
large number of qubits. This also implicitly tests that we've fixed the
excessive memory consumption because the memory requirements for an 8
qubit gate with the bug still present would not be runnable on most
current systems.

Co-authored-by: Jake Lishman <[email protected]>

* Add release note

---------

Co-authored-by: Matthew Treinish <[email protected]>
Co-authored-by: Jake Lishman <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
(cherry picked from commit 992d74f)
mergify bot added a commit that referenced this issue Mar 1, 2023
* Fix wrong argument supplied to _identity_op() #9197

* Add test case with large qubit gate

This commit adds a test case to ensure we get the correct result with a
large number of qubits. This also implicitly tests that we've fixed the
excessive memory consumption because the memory requirements for an 8
qubit gate with the bug still present would not be runnable on most
current systems.

Co-authored-by: Jake Lishman <[email protected]>

* Add release note

---------

Co-authored-by: Matthew Treinish <[email protected]>
Co-authored-by: Jake Lishman <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
(cherry picked from commit 992d74f)

Co-authored-by: M St <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
king-p3nguin pushed a commit to king-p3nguin/qiskit-terra that referenced this issue May 22, 2023
* Fix wrong argument supplied to _identity_op() Qiskit#9197

* Add test case with large qubit gate

This commit adds a test case to ensure we get the correct result with a
large number of qubits. This also implicitly tests that we've fixed the
excessive memory consumption because the memory requirements for an 8
qubit gate with the bug still present would not be runnable on most
current systems.

Co-authored-by: Jake Lishman <[email protected]>

* Add release note

---------

Co-authored-by: Matthew Treinish <[email protected]>
Co-authored-by: Jake Lishman <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.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

Successfully merging a pull request may close this issue.

2 participants