Refactor Rust crates to build a single extension module (#12134)

* Refactor Rust crates to build a single extension module

Previously we were building three different Python extension modules out
of Rust space.  This was convenient for logical separation of the code,
and for making the incremental compilations of the smaller crates
faster, but had the disadvantage that `qasm2` and `qasm3` couldn't
access the new circuit data structures directly from Rust space and had
to go via Python.

This modifies the crate structure to put the Rust-space acceleration
logic into crates that we only build as Rust libraries, then adds
another (`pyext`) crate to be the only that that actually builds as
shared Python C extension.  This then lets the Rust crates interact with
each other (and the Rust crates still contain PyO3 Python-binding logic
internally) and accept and output each others' Python types.

The one Python extension is still called `qiskit._accelerate`, but it's
built by the Rust crate `qiskit-pyext`.  This necessitated some further
changes, since `qiskit._accelerate` now contains acccelerators for lots
of parts of the Qiskit package hierarchy, but needs to have a single
low-level place to initialise itself:

- The circuit logic `circuit` is separated out into its own separate
  crate so that this can form the lowest part of the Rust hierarchy.
  This is done so that `accelerate` with its grab-bag of accelerators
  from all over the place does not need to be the lowest part of the
  stack.  Over time, it's likely that everything will start touching
  `circuit` anyway.

- `qiskit._qasm2` and `qiskit._qasm3` respectively became
  `qiskit._accelerate.qasm2` and `qiskit._accelerate.qasm3`, since they
  no longer get their own extension modules.

- `qasm3` no longer stores a Python object on itself during module
  initialisation that depended on `qiskit.circuit.library` to create.
  Now, the Python-space `qiskit.qasm3` does that and the `qasm3` crate
  just retrieves that at Python runtime, since that crate requires
  Qiskit to be importable anyway.

* Fix and document Rust test harness

* Fix new clippy complaints

These don't appear to be new from this patch series, but changing the
access modifiers on some of the crates seems to make clippy complain
more vociferously about some things.

* Fix pylint import order

* Remove erroneous comment

* Fix typos

Co-authored-by: Kevin Hartman <kevin@hart.mn>

* Comment on `extension-module`

* Unify naming of `qiskit-pyext`

* Remove out-of-date advice on starting new crates

---------

Co-authored-by: Kevin Hartman <kevin@hart.mn>

Cargo.toml

@@ -16,10 +16,19 @@ license = "Apache-2.0"
 [workspace.dependencies]
 indexmap.version = "2.2.6"
 hashbrown.version = "0.14.0"
-# This doesn't set `extension-module` as a shared feature because we need to be able to disable it
-# during Rust-only testing (see # https://github.com/PyO3/pyo3/issues/340).
+# Most of the crates don't need the feature `extension-module`, since only `qiskit-pyext` builds an
+# actual C extension (the feature disables linking in `libpython`, which is forbidden in Python
+# distributions).  We only activate that feature when building the C extension module; we still need
+# it disabled for Rust-only tests to avoid linker errors with it not being loaded.  See
+# https://pyo3.rs/main/features#extension-module for more.
 pyo3 = { version = "0.21.2", features = ["abi3-py38"] }
 
+# These are our own crates.
+qiskit-accelerate = { path = "crates/accelerate" }
+qiskit-circuit = { path = "crates/circuit" }
+qiskit-qasm2 = { path = "crates/qasm2" }
+qiskit-qasm3 = { path = "crates/qasm3" }
+
 [profile.release]
 lto = 'fat'
 codegen-units = 1

crates/README.md

@@ -0,0 +1,64 @@
+## Crate structure
+
+All the crates in here are called `qiskit-*`, and are stored in directories that omit the `qiskit-`.
+
+This crate structure currently serves the purpose of building only a single Python extension module, which still separates out some of the Rust code into separate logical chunks.
+The intention is that (much) longer term, we might be wanting to expose more of Qiskit functionality directly for other languages to interact with without going through Python.
+
+* `qiskit-pyext` is the only crate that actually builds a Python C extension library.
+  This is kind of like the parent crate of all the others, from an FFI perspective; others define `pyclass`es and `pyfunction`s and the like, but it's `qiskit-pyext` that builds the C extension.
+  Our C extension is built as `qiskit._accelerate` in Python space.
+* `qiskit-accelerate` is a catch-all crate for one-off accelerators.
+  If what you're working on is small and largely self-contained, you probably just want to put it in here, then bind it to the C extension module in `qiskit-pyext`.
+* `qiskit-circuit` is a base crate defining the Rust-space circuit objects.
+  This is one of the lowest points of the stack; everything that builds or works with circuits from Rust space depends on this.
+* `qiskit-qasm2` is the OpenQASM 2 parser.
+  This depends on `qiskit-circuit`, but is otherwise pretty standalone, and it's unlikely that other things will need to interact with it.
+* `qiskit-qasm3` is the Qiskit-specific side of the OpenQASM 3 importer.
+  The actual parser lives at https://github.com/Qiskit/openqasm3_parser, and is its own set of Rust-only crates.
+
+We use a structure with several crates in it for a couple of reasons:
+
+* logical separation of code
+* faster incremental compile times
+
+When we're doing Rust/Python interaction, though, we have to be careful.
+Pure-Rust FFI with itself over dynamic-library boundaries (like a Python C extension) isn't very natural, since Rust heavily prefers static linking.
+If we had more than one Python C extension, it would be very hard to interact between the code in them.
+This would be a particular problem for defining the circuit object and using it in other places, which is something we absolutely need to do.
+
+## Developer notes
+
+### Beware of initialisation order
+
+The Qiskit C extension `qiskit._accelerate` needs to be initialised in a single go.
+It is the lowest part of the Python package stack, so it cannot rely on importing other parts of the Python library at initialisation time (except for exceptions through PyO3's `import_exception!` mechanism).
+This is because, unlike pure-Python modules, the initialisation of `_accelerate` cannot be done partially, and many components of Qiskit import their accelerators from `_accelerate`.
+
+In general, this should not be too onerous a requirement, but if you violate it, you might see Rust panics on import, and PyO3 should wrap that up into an exception.
+You might be able to track down the Rust source of the import cycle by running the import with the environment variable `RUST_BACKTRACE=full`.
+
+
+### Tests
+
+Most of our functionality is tested through the Python-space tests of the `qiskit` Python package, since the Rust implementations are (to begin with) just private implementation details.
+However, where it's more useful, Rust crates can also include Rust-space tests within themselves.
+
+Each of the Rust crates disables `doctest` because our documentation tends to be written in Sphinx's rST rather than Markdown, so `rustdoc` has a habit of thinking various random bits of syntax are codeblocks.
+We can revisit that if we start writing crates that we intend to publish.
+
+#### Running the tests
+
+You need to make sure that the tests can build in standalone mode, i.e. that they link in `libpython`.
+By default, we activate PyO3's `extension-module` feature in `crates/pyext`, so you have to run with the tests with that disabled:
+
+```bash
+cargo test --no-default-features
+```
+
+On Linux, you might find that the dynamic linker still can't find `libpython`.
+If so, you can extend the environment variable `LD_LIBRARY_PATH` to include it:
+
+```bash
+export LD_LIBRARY_PATH="$(python -c 'import sys; print(sys.base_prefix)')/lib:$LD_LIBRARY_PATH"
+```

crates/accelerate/Cargo.toml

@@ -1,18 +1,13 @@
 [package]
-name = "qiskit_accelerate"
+name = "qiskit-accelerate"
 version.workspace = true
 edition.workspace = true
 rust-version.workspace = true
 license.workspace = true
 
 [lib]
 name = "qiskit_accelerate"
-crate-type = ["cdylib"]
-
-[features]
-# This is a test-only shim removable feature.  See the root `Cargo.toml`.
-default = ["extension-module"]
-extension-module = ["pyo3/extension-module"]
+doctest = false
 
 [dependencies]
 rayon = "1.10"
@@ -26,6 +21,7 @@ num-complex = "0.4"
 num-bigint = "0.4"
 rustworkx-core = "0.14"
 faer = "0.18.2"
+qiskit-circuit.workspace = true
 
 [dependencies.smallvec]
 version = "1.13"

crates/accelerate/README.md

@@ -1,21 +1,10 @@
-# `qiskit._accelerate`
+# `qiskit-accelerate`
 
-This crate provides a bits-and-pieces Python extension module for small, self-contained functions
+This crate provides a bits-and-pieces Rust libary for small, self-contained functions
 that are used by the main Python-space components to accelerate certain tasks.  If you're trying to
 speed up one particular Python function by replacing its innards with a Rust one, this is the best
-place to put the code.  This is _usually_ the right place to put Rust/Python interfacing code.
+place to put the code.  This is _usually_ the right place to put new Rust/Python code.
 
-The crate is made accessible as a private submodule, `qiskit._accelerate`.  There are submodules
-within that (largely matching the structure of the Rust code) mostly for grouping similar functions.
-
-Some examples of when it might be more appropriate to start a new crate instead of using the
-ready-made solution of `qiskit._accelerate`:
-
-* The feature you are developing will have a large amount of domain-specific Rust code and is a
-  large self-contained module.  If it reasonably works in a single Rust file, you probably just want
-  to put it here.
-
-* The Rust code is for re-use within other Qiskit crates and maintainability of the code will be
-  helped by using the crate system to provide API boundaries between the different sections.
-
-* You want to start writing your own procedural macros.
+The `qiskit-pyext` crate is what actually builds the C extension modules.  Modules in here should define
+themselves has being submodules of `qiskit._accelerate`, and then the `qiskit-pyext` crate should bind them
+into its `fn _accelerate` when it's making the C extension.

crates/accelerate/src/euler_one_qubit_decomposer.rs

@@ -19,6 +19,7 @@ use smallvec::{smallvec, SmallVec};
 use std::cmp::Ordering;
 use std::f64::consts::PI;
 use std::ops::Deref;
+use std::str::FromStr;
 
 use pyo3::exceptions::{PyIndexError, PyValueError};
 use pyo3::prelude::*;
@@ -30,7 +31,7 @@ use ndarray::prelude::*;
 use numpy::PyReadonlyArray2;
 use pyo3::pybacked::PyBackedStr;
 
-use crate::utils::SliceOrInt;
+use qiskit_circuit::SliceOrInt;
 
 pub const ANGLE_ZERO_EPSILON: f64 = 1e-12;
 
@@ -604,27 +605,31 @@ impl EulerBasis {
     }
 
     #[new]
-    pub fn from_str(input: &str) -> PyResult<Self> {
-        let res = match input {
-            "U321" => EulerBasis::U321,
-            "U3" => EulerBasis::U3,
-            "U" => EulerBasis::U,
-            "PSX" => EulerBasis::PSX,
-            "ZSX" => EulerBasis::ZSX,
-            "ZSXX" => EulerBasis::ZSXX,
-            "U1X" => EulerBasis::U1X,
-            "RR" => EulerBasis::RR,
-            "ZYZ" => EulerBasis::ZYZ,
-            "ZXZ" => EulerBasis::ZXZ,
-            "XYX" => EulerBasis::XYX,
-            "XZX" => EulerBasis::XZX,
-            basis => {
-                return Err(PyValueError::new_err(format!(
-                    "Invalid target basis '{basis}'"
-                )));
-            }
-        };
-        Ok(res)
+    pub fn __new__(input: &str) -> PyResult<Self> {
+        Self::from_str(input)
+            .map_err(|_| PyValueError::new_err(format!("Invalid target basis '{input}'")))
+    }
+}
+
+impl FromStr for EulerBasis {
+    type Err = ();
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        match s {
+            "U321" => Ok(EulerBasis::U321),
+            "U3" => Ok(EulerBasis::U3),
+            "U" => Ok(EulerBasis::U),
+            "PSX" => Ok(EulerBasis::PSX),
+            "ZSX" => Ok(EulerBasis::ZSX),
+            "ZSXX" => Ok(EulerBasis::ZSXX),
+            "U1X" => Ok(EulerBasis::U1X),
+            "RR" => Ok(EulerBasis::RR),
+            "ZYZ" => Ok(EulerBasis::ZYZ),
+            "ZXZ" => Ok(EulerBasis::ZXZ),
+            "XYX" => Ok(EulerBasis::XYX),
+            "XZX" => Ok(EulerBasis::XZX),
+            _ => Err(()),
+        }
     }
 }
 
@@ -714,7 +719,7 @@ pub fn unitary_to_gate_sequence(
 ) -> PyResult<Option<OneQubitGateSequence>> {
     let mut target_basis_vec: Vec<EulerBasis> = Vec::with_capacity(target_basis_list.len());
     for basis in target_basis_list {
-        let basis_enum = EulerBasis::from_str(basis.deref())?;
+        let basis_enum = EulerBasis::__new__(basis.deref())?;
         target_basis_vec.push(basis_enum)
     }
     let unitary_mat = unitary.as_array();