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

Fix replace_block_with_op on operations with wrong number of qubits #12637

Merged
merged 4 commits into from
Jul 1, 2024

Conversation

Cryoris
Copy link
Contributor

@Cryoris Cryoris commented Jun 21, 2024

Summary

Previously, DAGCircuit.replace_block_with_op allowed to place an
n-qubit operation onto a block of m qubits, leaving the DAG in an
invalid state. This behavior has been fixed, and the attempt will raise
a DAGCircuitError.

Details and comments

For example, this is currently possible:

from qiskit import transpile
from qiskit.circuit import QuantumCircuit
from qiskit.converters import circuit_to_dag, dag_to_circuit
from qiskit.circuit.library import XGate

# i am fine
qc = QuantumCircuit(2)
qc.x(range(2))

# a bit of dag mutilation
dag = circuit_to_dag(qc)
to_replace = list(dag.op_nodes())
new_node = XGate()
idx_map = {node.qargs[0]: i for i, node in enumerate(to_replace)}
dag.replace_block_with_op(to_replace, new_node, idx_map)

# the result
midlifecrisis = dag_to_circuit(dag)
print(midlifecrisis)

# naturally breaks
tqc = transpile(midlifecrisis, basis_gates=["u", "cx"])
print(tqc)

which prints

     ┌────┐
q_0: ┤0   ├
     │  X │
q_1: ┤1   ├
     └────┘
# and then breaks upon transpilation

@Cryoris Cryoris added stable backport potential The bug might be minimal and/or import enough to be port to stable Changelog: Bugfix Include in the "Fixed" section of the changelog labels Jun 21, 2024
@Cryoris Cryoris requested a review from a team as a code owner June 21, 2024 13:48
@qiskit-bot
Copy link
Collaborator

One or more of the following people are relevant to this code:

  • @Qiskit/terra-core

@Cryoris Cryoris changed the title fix illegal op insertion Fix replace_block_with_op on operations with wrong number of qubits Jun 21, 2024
@coveralls
Copy link

coveralls commented Jun 21, 2024

Pull Request Test Coverage Report for Build 9615440134

Details

  • 3 of 3 (100.0%) changed or added relevant lines in 2 files are covered.
  • 4 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.04%) to 89.759%

Files with Coverage Reduction New Missed Lines %
crates/qasm2/src/lex.rs 4 93.13%
Totals Coverage Status
Change from base Build 9614505164: 0.04%
Covered Lines: 63528
Relevant Lines: 70776

💛 - Coveralls

@ElePT
Copy link
Contributor

ElePT commented Jun 21, 2024

The times of the good old multi-X gate are over.

@coveralls
Copy link

coveralls commented Jun 24, 2024

Pull Request Test Coverage Report for Build 9647348633

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 3 of 3 (100.0%) changed or added relevant lines in 2 files are covered.
  • 96 unchanged lines in 6 files lost coverage.
  • Overall coverage increased (+0.05%) to 89.768%

Files with Coverage Reduction New Missed Lines %
qiskit/circuit/library/generalized_gates/permutation.py 1 92.86%
qiskit/circuit/library/standard_gates/xx_plus_yy.py 2 95.74%
qiskit/circuit/library/standard_gates/xx_minus_yy.py 2 95.74%
crates/qasm2/src/lex.rs 6 92.37%
crates/qasm2/src/parse.rs 6 97.15%
crates/circuit/src/operations.rs 79 73.24%
Totals Coverage Status
Change from base Build 9614505164: 0.05%
Covered Lines: 63661
Relevant Lines: 70917

💛 - Coveralls

@jlapeyre jlapeyre enabled auto-merge July 1, 2024 14:13
@jlapeyre jlapeyre added this pull request to the merge queue Jul 1, 2024
@Cryoris
Copy link
Contributor Author

Cryoris commented Jul 1, 2024

@Mergifyio backport stable/0.46 stable/1.1

Copy link
Contributor

mergify bot commented Jul 1, 2024

backport stable/0.46 stable/1.1

✅ Backports have been created

Merged via the queue into Qiskit:main with commit 67fd35a Jul 1, 2024
15 checks passed
mergify bot pushed a commit that referenced this pull request Jul 1, 2024
…#12637)

* fix illegal op insertion

* rm dangling print

* fix PauliEvolution

* Update qiskit/dagcircuit/dagcircuit.py

Co-authored-by: John Lapeyre <[email protected]>

---------

Co-authored-by: John Lapeyre <[email protected]>
(cherry picked from commit 67fd35a)
mergify bot pushed a commit that referenced this pull request Jul 1, 2024
…#12637)

* fix illegal op insertion

* rm dangling print

* fix PauliEvolution

* Update qiskit/dagcircuit/dagcircuit.py

Co-authored-by: John Lapeyre <[email protected]>

---------

Co-authored-by: John Lapeyre <[email protected]>
(cherry picked from commit 67fd35a)
github-merge-queue bot pushed a commit that referenced this pull request Jul 2, 2024
…#12637) (#12699)

* fix illegal op insertion

* rm dangling print

* fix PauliEvolution

* Update qiskit/dagcircuit/dagcircuit.py

Co-authored-by: John Lapeyre <[email protected]>

---------

Co-authored-by: John Lapeyre <[email protected]>
(cherry picked from commit 67fd35a)

Co-authored-by: Julien Gacon <[email protected]>
github-merge-queue bot pushed a commit that referenced this pull request Jul 2, 2024
…#12637) (#12700)

* fix illegal op insertion

* rm dangling print

* fix PauliEvolution

* Update qiskit/dagcircuit/dagcircuit.py

Co-authored-by: John Lapeyre <[email protected]>

---------

Co-authored-by: John Lapeyre <[email protected]>
(cherry picked from commit 67fd35a)

Co-authored-by: Julien Gacon <[email protected]>
Procatv pushed a commit to Procatv/qiskit-terra-catherines that referenced this pull request Aug 1, 2024
…Qiskit#12637)

* fix illegal op insertion

* rm dangling print

* fix PauliEvolution

* Update qiskit/dagcircuit/dagcircuit.py

Co-authored-by: John Lapeyre <[email protected]>

---------

Co-authored-by: John Lapeyre <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: Bugfix Include in the "Fixed" section of the changelog stable backport potential The bug might be minimal and/or import enough to be port to stable
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants