Skip to content

Add indexing of NLayout#14908

Merged
alexanderivrii merged 1 commit intoQiskit:mainfrom
jakelishman:sabre/layers/0
Mar 25, 2026
Merged

Add indexing of NLayout#14908
alexanderivrii merged 1 commit intoQiskit:mainfrom
jakelishman:sabre/layers/0

Conversation

@jakelishman
Copy link
Copy Markdown
Member

@jakelishman jakelishman commented Aug 14, 2025

This allows NLayout to be indexed by VirtualQubit and PhysicalQubit, which is a shorter way of writing VirtualQubit::to_phys and PhysicalQubit::to_virt.

@jakelishman jakelishman requested a review from a team as a code owner August 14, 2025 15:09
@jakelishman jakelishman added this to the 2.2.0 milestone Aug 14, 2025
@jakelishman jakelishman added the mod: transpiler Issues and PRs related to Transpiler label Aug 14, 2025
@qiskit-bot
Copy link
Copy Markdown
Collaborator

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

  • @Qiskit/terra-core

@coveralls
Copy link
Copy Markdown

Pull Request Test Coverage Report for Build 16969118917

Details

  • 4 of 10 (40.0%) changed or added relevant lines in 3 files are covered.
  • 19 unchanged lines in 5 files lost coverage.
  • Overall coverage decreased (-0.01%) to 88.247%

Changes Missing Coverage Covered Lines Changed/Added Lines %
crates/circuit/src/nlayout.rs 0 6 0.0%
Files with Coverage Reduction New Missed Lines %
crates/circuit/src/parameter/parameter_expression.rs 1 83.39%
crates/transpiler/src/passes/sabre/dag.rs 1 96.97%
crates/qasm2/src/lex.rs 3 92.53%
crates/qasm2/src/parse.rs 6 97.09%
crates/circuit/src/parameter/symbol_expr.rs 8 74.5%
Totals Coverage Status
Change from base Build 16967470304: -0.01%
Covered Lines: 87840
Relevant Lines: 99539

💛 - Coveralls

@raynelfss raynelfss modified the milestones: 2.2.0, 2.3.0 Aug 27, 2025
@github-project-automation github-project-automation Bot moved this to Ready in Qiskit 2.3 Oct 7, 2025
@debasmita2102
Copy link
Copy Markdown
Contributor

Just a naive question, in add_heuristic_layouts the dense trial now uses problem.dag.num_qubits() instead of the hardware width (num_physical_qubits) when calling compute_dense_starting_layout, so dummy ancillas are no longer included in this initial layout. Are we documenting any concrete changes in Sabre’s behaviour from this—either in runtime or in SWAP/depth quality—on the benchmark circuits, particularly for cases with many unused ancillas versus circuits that already occupy most of the device? Is that needed?

@Cryoris Cryoris removed this from Qiskit 2.3 Dec 12, 2025
@Cryoris Cryoris modified the milestones: 2.3.0, 2.4.0 Dec 12, 2025
@mtreinish mtreinish modified the milestones: 2.4.0, 2.5.0 Mar 19, 2026
This allows `NLayout` to be indexed by `VirtualQubit` and
`PhysicalQubit`, which is a shorter way of writing
`VirtualQubit::to_phys` and `PhysicalQubit::to_virt`.
@jakelishman jakelishman removed the on hold Can not fix yet label Mar 24, 2026
@jakelishman
Copy link
Copy Markdown
Member Author

This was previously dependent on #14605 for no particular reason, but I've uncoupled it to allow the Sabre-layers chain to move forwards, since #14605 is slightly contentious given the discussion of #14917.

It's now unblocked and can proceed.

@jakelishman jakelishman added Changelog: None Do not include in the GitHub Release changelog. Rust This PR or issue is related to Rust code in the repository labels Mar 24, 2026
@jakelishman
Copy link
Copy Markdown
Member Author

Just a naive question, in add_heuristic_layouts the dense trial now uses problem.dag.num_qubits() instead of the hardware width (num_physical_qubits) when calling compute_dense_starting_layout, so dummy ancillas are no longer included in this initial layout. Are we documenting any concrete changes in Sabre’s behaviour from this—either in runtime or in SWAP/depth quality—on the benchmark circuits, particularly for cases with many unused ancillas versus circuits that already occupy most of the device? Is that needed?

Oh, I never saw this at the time, sorry. If that change was showing up in this PR, it must have been because it was in a chain before. The PR that made the change you're talking about is #14604, which did come with a release note.

Copy link
Copy Markdown
Member

@alexanderivrii alexanderivrii left a comment

Choose a reason for hiding this comment

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

Neat feature that improves ergonomics - LGTM!

@alexanderivrii alexanderivrii added this pull request to the merge queue Mar 25, 2026
Merged via the queue into Qiskit:main with commit 03e8f5b Mar 25, 2026
27 checks passed
@github-project-automation github-project-automation Bot moved this from Ready to Done in Qiskit 2.4 Mar 25, 2026
@jakelishman jakelishman deleted the sabre/layers/0 branch March 25, 2026 13:21
@github-project-automation github-project-automation Bot moved this from Ready to Done in Qiskit 2.5 Apr 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Changelog: None Do not include in the GitHub Release changelog. mod: transpiler Issues and PRs related to Transpiler Rust This PR or issue is related to Rust code in the repository

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

9 participants