Add `qk_obs_apply_layout` to apply layouts to `QkObs` #14106

Cryoris · 2025-03-26T16:13:50Z

Summary

This PR adds support to apply layouts to QkObs in the C API.

Details and comments

This is done by exposing NLayout to represent the layout, which we also expect to be the result type of routing passes like VF2Layout. It would also be possible to use plain &[u32] but this way qk_obs_apply_layout always uses a consistent type. The layout is exposed as opaque pointer called QkLayout with associated functions qk_layout_new and qk_layout_free.

qiskit-bot · 2025-03-26T16:13:55Z

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

@Qiskit/terra-core

Cryoris · 2025-03-26T16:15:14Z

crates/accelerate/src/nlayout.rs

 pub struct NLayout {
-    virt_to_phys: Vec<PhysicalQubit>,
-    phys_to_virt: Vec<VirtualQubit>,
+    pub virt_to_phys: Vec<PhysicalQubit>,


These are made public to be able to easily call SparseObservable::apply_layout, which takes the virt_to_phys vector. This could be worked around by using the available public methods, but I think that would require a copy that I wanted to avoid.

Cryoris · 2025-03-26T16:16:30Z

crates/cext/src/sparse_observable.rs

+
+    // Cast the slice of PhysicalQubit (which is u32) to plain u32, which is required
+    // by the apply_layout method.
+    let as_slice: &[u32] = ::bytemuck::cast_slice(&layout.virt_to_phys);


This line (and the NoUninit for PhysicalQubit) warrants extra care since it's the first time I'm using bytemuck 😄

crates/cext/src/layout.rs

coveralls · 2025-03-26T16:41:18Z

Pull Request Test Coverage Report for Build 17462696448

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.

For more information on this, see Tracking coverage changes with pull request builds.
To avoid this issue with future PRs, see these Recommended CI Configurations.
For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

18 of 21 (85.71%) changed or added relevant lines in 1 file are covered.
1174 unchanged lines in 20 files lost coverage.
Overall coverage increased (+0.02%) to 88.389%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
crates/cext/src/sparse_observable.rs	18	21	85.71%

Files with Coverage Reduction	New Missed Lines	%
crates/circuit/src/parameter/parameter_expression.rs	1	82.79%
crates/qasm2/src/lex.rs	4	91.75%
crates/cext/src/sparse_observable.rs	7	89.96%
qiskit/qasm3/ast.py	7	95.34%
crates/circuit/src/packed_instruction.rs	9	93.39%
qiskit/user_config.py	9	85.45%
crates/transpiler/src/commutation_checker.rs	13	88.46%
qiskit/visualization/circuit/_utils.py	15	95.62%
crates/cext/src/transpiler/transpile_function.rs	16	60.24%
crates/transpiler/src/passes/vf2/vf2_layout.rs	16	91.18%

Totals
Change from base Build 17427735527:	0.02%
Covered Lines:	92153
Relevant Lines:	104258

💛 - Coveralls

jakelishman

Honestly, exposing NLayout like this feels premature to me. The low-level Rust interface to SparseObservable::apply_layout doesn't take NLayout, only layout: Option<&[u32]>, num_qubits: u32, and NLayout doesn't represent everything that TranspileLayout does. Right now, C still has to completely manually build the NLayout, so if anything, exposing NLayout makes it harder, not easier.

I'd suggest we wait til we actually know what we're going to be returning from any form of the transpiler. NLayout might be accessible, but the "high-level" return type from the transpiler representing layouts is still presumably going to be a TranspileLayout-like wrapper. SparseObservable.apply_layout doesn't take a Layout in Python space, which is the analogue.

Cryoris · 2025-03-27T08:41:26Z

Just using an array instead of NLayout would also be fine, the idea was to make it very ergonomic to use in a workflow where you'd just call e.g. VF2Layout and feed the output into qk_obs_apply_layout. Since #14056 returns something that is almost NLayout that seemed like a good candidate 😄

But won't we have to introduce equivalents of both Layout and TranspileLayout anyways, since the former is what the layouting passes generate and the latter what we associate with the final circuit?

jakelishman · 2025-03-27T09:03:12Z

We'll likely need something akin to TranspileLayout at least. VF2 does produce something like an NLayout, but VF2 isn't the whole story - the layout stage only finds the initial layout of a circuit, but apply_layout on operators depends on both that and any induced routing permutation, the output of VF2 won't necessarily be what we want here.

jakelishman · 2025-03-27T09:04:18Z

My point is: we're not at a point yet where we can know what will be ergonomic. Let's not guess - there's no harm in presenting a low-level interface and then waiting a bit til we're more sure to present a high-level one.

This commit adds a Rust TranspileLayout object which is analgous to the Python space object. It is a self contained struct that is only for Rust and will be used by the C API for the C transpiler so that users will be able to reason about the permutations caused by the transpiler. This will also be used by Qiskit#14106 to apply a transpilation layout to the SparseObservable in C. This commit only adds the rust struct and it's associated tests, a subsequent PR will add the C API on top. This won't be used until we have a full path transpiler in C though (see Qiskit#14760).

* Add TranspileLayout struct to rust This commit adds a Rust TranspileLayout object which is analgous to the Python space object. It is a self contained struct that is only for Rust and will be used by the C API for the C transpiler so that users will be able to reason about the permutations caused by the transpiler. This will also be used by #14106 to apply a transpilation layout to the SparseObservable in C. This commit only adds the rust struct and it's associated tests, a subsequent PR will add the C API on top. This won't be used until we have a full path transpiler in C though (see #14760). * Add routing permutation composition method * Improve routing permutation APIs and make more rust native This commit improves the routing permutation method to make them more rust native. It updates the return to be a slice of PhysicalQubits instead of an owned vec. To get the explicit trivial permutation in the case of None a new method explicit_routing_permutation is added that returns `Cow<[PhysicalQubits]>` and only allocates in the None case. * Update final layout methods This commit updates the final layout method to return a NLayout and a new method final_index_layout is added to return a Vec. * Improve documentation * Fix lint * Add support for python interop This commit adds methods to convert to and from the Python space transpile layout class. This requires changing the data in the rust struct slightly so that we are tracking the virtual bits to build the view that the Python struct needs. This isn't strictly needed for a pure rust view of the data, but doesn't add much overhead to track it to support python. * Fix lint * Make initial_layout optional This commit updates the initial_layout attribute to make it optional. In the transpiler a layout doesn't necessarily need to be set so to account for that this the attribute is made optional. * Rename routing_permutation -> output_permutation * Make output_permutation use Qubit instead of PhysicalQubit Now that output_permutation is storing the abstract permutation whether there is a layout or not it means the PhysicalQubit type doesn't accurately reflect the purpose of the field anymore since it might not be a physical qubit. This commit fixes this by using the Qubit type which just represent the index in the circuit which is all the permutation is. * Fix Python accesses in `TranspileLayout::from_py_native` --------- Co-authored-by: Jake Lishman <[email protected]>

mtreinish

This needs a rebase pretty badly, it predates the crate re-organization apparently since nlayout and sabre layout are still in accelerate. Besides the rebase I think this PR can be greatly simplified now that we have TranspileLayout in rust and C. You can just use that directly as an input for your apply_layout() function and it's all pretty straightforward from there.

crates/cext/src/sparse_observable.rs

releasenotes/notes/qk-layout-df9805232e7d6ca4.yaml

crates/cext/src/sparse_observable.rs

jakelishman · 2025-08-21T19:25:13Z

Besides the rebase I think this PR can be greatly simplified now that we have TranspileLayout in rust and C.

That's why this PR had no movement for several months and is way out of date - it was waiting for TranspileLayout to arrive.

jakelishman · 2025-08-22T11:59:20Z

Julien and I talked offline, and we think that going to:

QkObs *qk_obs_apply_layout(const QkObs *obs, uint32_t *layout, uint32_t num_qubits);

is the best trade-off for C, with the option to add a variant

QkObs *qk_obs_apply_transpile_layout(const QkObs *obs, const QkTranspileLayout *layout);

later if there's direct demand.

The action on SparseObservable fundamentally is just a single relabelling, which is a useful operation outside of the context of compilation as well. Since this PR was opened, we've gained a lot more formalism and understanding of how to teach TranspileLayout stuff, to the degree that we can clearly say that the (Rust-space) action that you want here is

let obs: SparseObservable;
let layout: TranspileLayout;
let new_obs = obs.apply_layout(&layout.final_index_layout(false), layout.num_output_qubits());

and C space has direct equivalents. I think this is simple enough that we can simply teach it by documentation, and that gives C more control over its allocations and low-level access to the SparseObservable relabelling outside the context of the compiler.

mtreinish · 2025-08-22T12:27:06Z

crates/cext/src/sparse_observable.rs

+
+    let obs_with_layout = match obs.apply_layout(layout, num_qubits) {
+        Ok(obs_with_layout) => obs_with_layout,
+        Err(_) => return ExitCode::TranspilerError,


Is this actually a transpiler error if the user is providing a pointer to the layout array? It's more the input is invalid and probably came from the transpiler but they could have just generated an invalid layout right?

Yeah good question, it uses CoherenceError Rust side, saying e.g. that the qubit index is invalid. The nicest thing would be to match the error variants and give some actual information.

crates/cext/src/sparse_observable.rs

mtreinish

I think this is good to go now, just one small comment about the docs for the new function besides that I think we can move forward with this.

mtreinish · 2025-09-02T20:26:31Z

crates/cext/src/sparse_observable.rs

+/// @param layout A pointer to the layout. The pointer must point to an array to
+/// ``qk_obs_num_qubits(obs)`` elements of type ``uint32_t``. Each element must have values
+/// in ``[0, num_qubits)``.


What does the array passed to layout represent? I assume it's mapping index i to the physical qubit. But it's good to be explicit here about what the alayout array is besides just the data types.

good point, I added an explanation to the docstring in cf65b46 👍🏻

mtreinish

I was going to approve it but I caught a few more small things.

mtreinish · 2025-09-03T14:25:57Z

test/c/test_sparse_observable.c

    num_failed += RUN_TEST(test_direct_fail);
    num_failed += RUN_TEST(test_obs_str);
    num_failed += RUN_TEST(test_obsterm_str);
+    num_failed += RUN_TEST(test_apply_layout);


Suggested change

num_failed += RUN_TEST(test_apply_layout);

num_failed += RUN_TEST(test_apply_layout);

num_failed += RUN_TEST(test_apply_layout_too_small);

Uff good catch! We should add a bash script that checks this...

Fixed 👍🏻

If we get in the habit of declaring our test functions as static, I think the linter should warn about it being unused.

mtreinish · 2025-09-03T14:26:15Z

crates/circuit/src/nlayout.rs

        layout.phys_to_virt[self.index()]
    }
 }
+unsafe impl ::bytemuck::NoUninit for PhysicalQubit {}


I don't think this is actually needed anymore (not sure when it was). Testing it locally nothing fails if you remove this.

Suggested change

unsafe impl ::bytemuck::NoUninit for PhysicalQubit {}

This was needed since I wanted to cast u32 to PhysicalQubit using bytemuck -- but we don't need that anymore 🙂

mtreinish · 2025-09-03T14:28:50Z

test/c/test_sparse_observable.c

+    int err = qk_obs_apply_layout(obs, layout, 3000);
+    qk_obs_free(obs);
+
+    return err == QkExitCode_MismatchedQubits ? Ok : EqualityError;


I think this is going to fail, in my local testing it's returning an IndexErrror

Oh yeah the MismatchedQubits would only be raised if the layout was too small to accommodate the qubits -- here we don't have enough indices instead. Fixed!

- NoUninit no longer needed - missed a test

* Add TranspileLayout struct to rust This commit adds a Rust TranspileLayout object which is analgous to the Python space object. It is a self contained struct that is only for Rust and will be used by the C API for the C transpiler so that users will be able to reason about the permutations caused by the transpiler. This will also be used by Qiskit#14106 to apply a transpilation layout to the SparseObservable in C. This commit only adds the rust struct and it's associated tests, a subsequent PR will add the C API on top. This won't be used until we have a full path transpiler in C though (see Qiskit#14760). * Add routing permutation composition method * Improve routing permutation APIs and make more rust native This commit improves the routing permutation method to make them more rust native. It updates the return to be a slice of PhysicalQubits instead of an owned vec. To get the explicit trivial permutation in the case of None a new method explicit_routing_permutation is added that returns `Cow<[PhysicalQubits]>` and only allocates in the None case. * Update final layout methods This commit updates the final layout method to return a NLayout and a new method final_index_layout is added to return a Vec. * Improve documentation * Fix lint * Add support for python interop This commit adds methods to convert to and from the Python space transpile layout class. This requires changing the data in the rust struct slightly so that we are tracking the virtual bits to build the view that the Python struct needs. This isn't strictly needed for a pure rust view of the data, but doesn't add much overhead to track it to support python. * Fix lint * Make initial_layout optional This commit updates the initial_layout attribute to make it optional. In the transpiler a layout doesn't necessarily need to be set so to account for that this the attribute is made optional. * Rename routing_permutation -> output_permutation * Make output_permutation use Qubit instead of PhysicalQubit Now that output_permutation is storing the abstract permutation whether there is a layout or not it means the PhysicalQubit type doesn't accurately reflect the purpose of the field anymore since it might not be a physical qubit. This commit fixes this by using the Qubit type which just represent the index in the circuit which is all the permutation is. * Fix Python accesses in `TranspileLayout::from_py_native` --------- Co-authored-by: Jake Lishman <[email protected]>

* apply transpile layout * raw interface * docs * clippy + memleak * add reno * review comments * fix tests * variable with arrays not valid for GCC/MSVC * explain the ``layout`` array better * Review comments - NoUninit no longer needed - missed a test

* Add TranspileLayout struct to rust This commit adds a Rust TranspileLayout object which is analgous to the Python space object. It is a self contained struct that is only for Rust and will be used by the C API for the C transpiler so that users will be able to reason about the permutations caused by the transpiler. This will also be used by Qiskit#14106 to apply a transpilation layout to the SparseObservable in C. This commit only adds the rust struct and it's associated tests, a subsequent PR will add the C API on top. This won't be used until we have a full path transpiler in C though (see Qiskit#14760). * Add routing permutation composition method * Improve routing permutation APIs and make more rust native This commit improves the routing permutation method to make them more rust native. It updates the return to be a slice of PhysicalQubits instead of an owned vec. To get the explicit trivial permutation in the case of None a new method explicit_routing_permutation is added that returns `Cow<[PhysicalQubits]>` and only allocates in the None case. * Update final layout methods This commit updates the final layout method to return a NLayout and a new method final_index_layout is added to return a Vec. * Improve documentation * Fix lint * Add support for python interop This commit adds methods to convert to and from the Python space transpile layout class. This requires changing the data in the rust struct slightly so that we are tracking the virtual bits to build the view that the Python struct needs. This isn't strictly needed for a pure rust view of the data, but doesn't add much overhead to track it to support python. * Fix lint * Make initial_layout optional This commit updates the initial_layout attribute to make it optional. In the transpiler a layout doesn't necessarily need to be set so to account for that this the attribute is made optional. * Rename routing_permutation -> output_permutation * Make output_permutation use Qubit instead of PhysicalQubit Now that output_permutation is storing the abstract permutation whether there is a layout or not it means the PhysicalQubit type doesn't accurately reflect the purpose of the field anymore since it might not be a physical qubit. This commit fixes this by using the Qubit type which just represent the index in the circuit which is all the permutation is. * Fix Python accesses in `TranspileLayout::from_py_native` --------- Co-authored-by: Jake Lishman <[email protected]>

* apply transpile layout * raw interface * docs * clippy + memleak * add reno * review comments * fix tests * variable with arrays not valid for GCC/MSVC * explain the ``layout`` array better * Review comments - NoUninit no longer needed - missed a test

Cryoris added Changelog: New Feature Include in the "Added" section of the changelog C API Related to the C API labels Mar 26, 2025

Cryoris requested a review from a team as a code owner March 26, 2025 16:13

Cryoris commented Mar 26, 2025

View reviewed changes

crates/cext/src/layout.rs Outdated Show resolved Hide resolved

jakelishman reviewed Mar 26, 2025

View reviewed changes

eliarbel linked an issue Apr 24, 2025 that may be closed by this pull request

Add qk_obs_apply_layout #14095

Closed

eliarbel added this to the 2.1.0 milestone Apr 29, 2025

eliarbel assigned Cryoris Apr 29, 2025

eliarbel modified the milestones: 2.1.0, 2.2.0 May 11, 2025

raynelfss added the on hold Can not fix yet label Jul 17, 2025

mtreinish mentioned this pull request Jul 22, 2025

Add TranspileLayout struct to rust #14778

Merged

mtreinish reviewed Aug 21, 2025

View reviewed changes

apply transpile layout

94bb9ba

Cryoris added 3 commits August 22, 2025 14:02

raw interface

82e55f9

docs

73a5bef

clippy + memleak

663371f

Cryoris force-pushed the qk-layout branch from cb68030 to 663371f Compare August 22, 2025 12:20

mtreinish removed the on hold Can not fix yet label Aug 22, 2025

add reno

1e004db

Cryoris changed the title ~~Add qk_obs_apply_layout plus necessary tooling~~ Add qk_obs_apply_layout to apply layouts to QkObs Aug 22, 2025

mtreinish reviewed Aug 22, 2025

View reviewed changes

Cryoris added 4 commits August 22, 2025 14:52

review comments

e896859

fix tests

0a30b5f

Merge branch 'main' into qk-layout

2bcc084

variable with arrays not valid for GCC/MSVC

5715ea3

mtreinish assigned mtreinish and jakelishman and unassigned Cryoris Aug 28, 2025

mtreinish reviewed Sep 2, 2025

View reviewed changes

Cryoris added 2 commits September 3, 2025 13:37

Merge branch 'main' into qk-layout

698a363

explain the layout array better

cf65b46

mtreinish reviewed Sep 3, 2025

View reviewed changes

Review comments

b9af921

- NoUninit no longer needed - missed a test

mtreinish approved these changes Sep 4, 2025

View reviewed changes

mtreinish added this pull request to the merge queue Sep 4, 2025

Merged via the queue into Qiskit:main with commit e86a050 Sep 4, 2025
26 checks passed

Cryoris deleted the qk-layout branch September 4, 2025 14:26

	num_failed += RUN_TEST(test_apply_layout);
	num_failed += RUN_TEST(test_apply_layout);
	num_failed += RUN_TEST(test_apply_layout_too_small);

Add qk_obs_apply_layout to apply layouts to QkObs #14106

Add qk_obs_apply_layout to apply layouts to QkObs #14106

Uh oh!

Conversation

Cryoris commented Mar 26, 2025

Summary

Details and comments

Uh oh!

qiskit-bot commented Mar 26, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coveralls commented Mar 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 17462696448

Warning: This coverage report may be inaccurate.

Details

💛 - Coveralls

Uh oh!

jakelishman left a comment

Choose a reason for hiding this comment

Uh oh!

Cryoris commented Mar 27, 2025

Uh oh!

jakelishman commented Mar 27, 2025

Uh oh!

jakelishman commented Mar 27, 2025

Uh oh!

mtreinish left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jakelishman commented Aug 21, 2025

Uh oh!

jakelishman commented Aug 22, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mtreinish left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mtreinish left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Cryoris Sep 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Add `qk_obs_apply_layout` to apply layouts to `QkObs` #14106

Add `qk_obs_apply_layout` to apply layouts to `QkObs` #14106

coveralls commented Mar 26, 2025 •

edited

Loading

Cryoris Sep 4, 2025 •

edited

Loading

Cryoris Sep 4, 2025 •

edited

Loading