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

GenericBackendV2 should fail when the backend cannot allocate the basis gate because its size #12653

Merged
merged 5 commits into from
Jun 26, 2024

Conversation

1ucian0
Copy link
Member

@1ucian0 1ucian0 commented Jun 25, 2024

Summary

GenericBackendV2 should not accept backends that are too small for certain gates. The following examples should be rejected:

GenericBackendV2(num_qubits=1, basis_gates=["cx", "id"])

GenericBackendV2(num_qubits=2, basis_gates=["ccx", "id"])

@ElePT found this!

1ucian0 and others added 2 commits June 25, 2024 10:45
@1ucian0 1ucian0 requested review from jyu00 and a team as code owners June 25, 2024 08:50
@qiskit-bot
Copy link
Collaborator

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

  • @Qiskit/terra-core

@1ucian0 1ucian0 added the Changelog: Bugfix Include in the "Fixed" section of the changelog label Jun 25, 2024
@coveralls
Copy link

coveralls commented Jun 25, 2024

Pull Request Test Coverage Report for Build 9659396940

Details

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • 23 unchanged lines in 3 files lost coverage.
  • Overall coverage decreased (-0.03%) to 89.729%

Files with Coverage Reduction New Missed Lines %
crates/qasm2/src/expr.rs 1 94.02%
crates/qasm2/src/lex.rs 4 92.62%
crates/qasm2/src/parse.rs 18 96.69%
Totals Coverage Status
Change from base Build 9650973845: -0.03%
Covered Lines: 63758
Relevant Lines: 71056

💛 - Coveralls

ElePT
ElePT previously approved these changes Jun 25, 2024
Copy link
Contributor

@ElePT ElePT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @1ucian0 LGTM, I left a small reno suggestion (am I allowed to approve?)

@coveralls
Copy link

coveralls commented Jun 25, 2024

Pull Request Test Coverage Report for Build 9663589789

Details

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • 7 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.01%) to 89.732%

Files with Coverage Reduction New Missed Lines %
crates/qasm2/src/expr.rs 1 94.02%
crates/qasm2/src/lex.rs 6 91.6%
Totals Coverage Status
Change from base Build 9661630219: 0.01%
Covered Lines: 63760
Relevant Lines: 71056

💛 - Coveralls

ElePT
ElePT previously approved these changes Jun 25, 2024
@ElePT ElePT enabled auto-merge June 25, 2024 14:06
@1ucian0
Copy link
Member Author

1ucian0 commented Jun 25, 2024

am I allowed to approve?

It seems you are.

@coveralls
Copy link

coveralls commented Jun 25, 2024

Pull Request Test Coverage Report for Build 9663987131

Details

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • 5 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.01%) to 89.731%

Files with Coverage Reduction New Missed Lines %
crates/qasm2/src/expr.rs 1 94.02%
crates/qasm2/src/lex.rs 4 92.37%
Totals Coverage Status
Change from base Build 9661630219: 0.01%
Covered Lines: 63759
Relevant Lines: 71056

💛 - Coveralls

@ElePT
Copy link
Contributor

ElePT commented Jun 25, 2024

I think we need to add an internal path for building a target for 1 qubit, because GenericBackendV2 contains 2q basis gates by default (that's why the test fails currently).

@coveralls
Copy link

coveralls commented Jun 25, 2024

Pull Request Test Coverage Report for Build 9667579174

Details

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • 6 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.03%) to 89.748%

Files with Coverage Reduction New Missed Lines %
crates/qasm2/src/lex.rs 6 92.62%
Totals Coverage Status
Change from base Build 9661630219: 0.03%
Covered Lines: 63771
Relevant Lines: 71056

💛 - Coveralls

@1ucian0
Copy link
Member Author

1ucian0 commented Jun 25, 2024

I think we need to add an internal path for building a target for 1 qubit, because GenericBackendV2 contains 2q basis gates by default (that's why the test fails currently).

I think I prefer to keep the default simple (without different values depending on the number of qubits) and be explicit on the basis _gate (see 43a94f1). What do you think?

@ElePT
Copy link
Contributor

ElePT commented Jun 26, 2024

I think that can work too. If we see this is causing problems we can always correct the 1q path.

@ElePT ElePT added this pull request to the merge queue Jun 26, 2024
@1ucian0
Copy link
Member Author

1ucian0 commented Jun 26, 2024

@Mergifyio backport stable/0.46 stable/1.1

Copy link
Contributor

mergify bot commented Jun 26, 2024

backport stable/0.46 stable/1.1

✅ Backports have been created

Merged via the queue into Qiskit:main with commit e36027c Jun 26, 2024
15 checks passed
mergify bot pushed a commit that referenced this pull request Jun 26, 2024
…is gate because its size (#12653)

* GenericBackendV2 should fail when the backend cannot allocate the basis gate because its size

Co-authored-by: Elena Peña Tapia <[email protected]>

* reno

* Update releasenotes/notes/fixes_GenericBackendV2-668e40596e1f070d.yaml

Co-authored-by: Elena Peña Tapia <[email protected]>

* another single qubit backend

---------

Co-authored-by: Elena Peña Tapia <[email protected]>
(cherry picked from commit e36027c)
mergify bot pushed a commit that referenced this pull request Jun 26, 2024
…is gate because its size (#12653)

* GenericBackendV2 should fail when the backend cannot allocate the basis gate because its size

Co-authored-by: Elena Peña Tapia <[email protected]>

* reno

* Update releasenotes/notes/fixes_GenericBackendV2-668e40596e1f070d.yaml

Co-authored-by: Elena Peña Tapia <[email protected]>

* another single qubit backend

---------

Co-authored-by: Elena Peña Tapia <[email protected]>
(cherry picked from commit e36027c)

# Conflicts:
#	qiskit/providers/fake_provider/generic_backend_v2.py
#	test/visual/mpl/graph/test_graph_matplotlib_drawer.py
github-merge-queue bot pushed a commit that referenced this pull request Jun 26, 2024
…is gate because its size (#12653) (#12667)

* GenericBackendV2 should fail when the backend cannot allocate the basis gate because its size

Co-authored-by: Elena Peña Tapia <[email protected]>

* reno

* Update releasenotes/notes/fixes_GenericBackendV2-668e40596e1f070d.yaml

Co-authored-by: Elena Peña Tapia <[email protected]>

* another single qubit backend

---------

Co-authored-by: Elena Peña Tapia <[email protected]>
(cherry picked from commit e36027c)

Co-authored-by: Luciano Bello <[email protected]>
github-merge-queue bot pushed a commit that referenced this pull request Jun 26, 2024
…is gate because its size (backport #12653) (#12666)

* GenericBackendV2 should fail when the backend cannot allocate the basis gate because its size (#12653)

* GenericBackendV2 should fail when the backend cannot allocate the basis gate because its size

Co-authored-by: Elena Peña Tapia <[email protected]>

* reno

* Update releasenotes/notes/fixes_GenericBackendV2-668e40596e1f070d.yaml

Co-authored-by: Elena Peña Tapia <[email protected]>

* another single qubit backend

---------

Co-authored-by: Elena Peña Tapia <[email protected]>
(cherry picked from commit e36027c)

# Conflicts:
#	qiskit/providers/fake_provider/generic_backend_v2.py
#	test/visual/mpl/graph/test_graph_matplotlib_drawer.py

* Fix Conflict

* Update test_graph_matplotlib_drawer.py

* black

---------

Co-authored-by: Luciano Bello <[email protected]>
Co-authored-by: Elena Peña Tapia <[email protected]>
Procatv pushed a commit to Procatv/qiskit-terra-catherines that referenced this pull request Aug 1, 2024
…is gate because its size (Qiskit#12653)

* GenericBackendV2 should fail when the backend cannot allocate the basis gate because its size

Co-authored-by: Elena Peña Tapia <[email protected]>

* reno

* Update releasenotes/notes/fixes_GenericBackendV2-668e40596e1f070d.yaml

Co-authored-by: Elena Peña Tapia <[email protected]>

* another single qubit backend

---------

Co-authored-by: Elena Peña Tapia <[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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants