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

Ensure translation stage always outputs ISA circuits #13792

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jakelishman
Copy link
Member

Summary

This shifts the responsibility of the translation stage slightly to always output ISA circuits. It previously was not 100% required that they did this, at least implicitly because Qiskit's built-in plugins didn't always respect 2q direction.

The builtin translation plugins now always respect 2q direction, which removes the need for the pre_optimization implicit stage, freeing it up for better user customisation.

This (in theory) shouldn't have runtime impacts because the optimisation loop was already having to do this afterwards anyway. For potential plugins that do respect gate direction (like the upcoming BasisConstructor), it can be a speedup in the default pipeline, since they won't need to run the gate-direction check any more.

Details and comments

Built on top of #13620, since it will need to change documentation in that PR.

Fix #13787.

@jakelishman jakelishman added Changelog: Deprecation Include in "Deprecated" section of changelog Changelog: API Change Include in the "Changed" section of the changelog mod: transpiler Issues and PRs related to Transpiler labels Feb 5, 2025
@jakelishman jakelishman added this to the 2.0.0 milestone Feb 5, 2025
@jakelishman jakelishman requested a review from a team as a code owner February 5, 2025 18:22
@qiskit-bot
Copy link
Collaborator

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

  • @Qiskit/terra-core

@jakelishman jakelishman added the on hold Can not fix yet label Feb 5, 2025
@coveralls
Copy link

coveralls commented Feb 5, 2025

Pull Request Test Coverage Report for Build 13168271554

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

  • 10 of 10 (100.0%) changed or added relevant lines in 1 file are covered.
  • 15 unchanged lines in 3 files lost coverage.
  • Overall coverage decreased (-0.005%) to 88.646%

Files with Coverage Reduction New Missed Lines %
crates/accelerate/src/euler_one_qubit_decomposer.rs 1 91.5%
crates/qasm2/src/lex.rs 6 92.23%
qiskit/transpiler/preset_passmanagers/common.py 8 85.94%
Totals Coverage Status
Change from base Build 13165103262: -0.005%
Covered Lines: 79222
Relevant Lines: 89369

💛 - Coveralls

This shifts the responsibility of the `translation` stage slightly to
always output ISA circuits.  It previously was not 100% required that
they did this, at least implicitly because Qiskit's built-in plugins
didn't always respect 2q direction.

The builtin translation plugins now always respect 2q direction, which
removes the need for the `pre_optimization` implicit stage, freeing it
up for better user customisation.

This (in theory) shouldn't have runtime impacts because the optimisation
loop was already having to do this afterwards anyway.  For potential
plugins that _do_ respect gate direction (like the upcoming
`BasisConstructor`), it can be a speedup in the default pipeline, since
they won't need to run the gate-direction check any more.
@jakelishman jakelishman force-pushed the translation-stage-direction branch from 0caf0c9 to 92f30cd Compare February 5, 2025 23:08
@jakelishman jakelishman removed the on hold Can not fix yet label Feb 5, 2025
@jakelishman
Copy link
Member Author

jakelishman commented Feb 5, 2025

Now rebased over main. I'll run the benchmark suite overnight.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: API Change Include in the "Changed" section of the changelog Changelog: Deprecation Include in "Deprecated" section of changelog mod: transpiler Issues and PRs related to Transpiler
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move GateDirection responsibility into translation preset stage
3 participants