-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Add qk_obs_apply_layout
plus necessary tooling
#14106
base: main
Are you sure you want to change the base?
Conversation
This exposes ``NLayout`` to represent the layout, which we expect to be the result type of routing passes. It would also be possible to use plain &[u32] but this way ``qk_obs_apply_layout`` always uses a consistent type.
One or more of the following people are relevant to this code:
|
@@ -91,8 +93,8 @@ impl VirtualQubit { | |||
#[pyclass(module = "qiskit._accelerate.nlayout")] | |||
#[derive(Clone, Debug)] | |||
pub struct NLayout { | |||
virt_to_phys: Vec<PhysicalQubit>, | |||
phys_to_virt: Vec<VirtualQubit>, | |||
pub virt_to_phys: Vec<PhysicalQubit>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
|
||
// 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); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line (and the NoUninit
for PhysicalQubit
) warrants extra care since it's the first time I'm using bytemuck
😄
Pull Request Test Coverage Report for Build 14092960259Details
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Just using an array instead of But won't we have to introduce equivalents of both |
We'll likely need something akin to |
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. |
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 wayqk_obs_apply_layout
always uses a consistent type. The layout is exposed as opaque pointer calledQkLayout
with associated functionsqk_layout_new
andqk_layout_free
.