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

Add a rust representation of operations, instructions, and gates #12205

Closed
mtreinish opened this issue Apr 18, 2024 · 1 comment · Fixed by #12459
Closed

Add a rust representation of operations, instructions, and gates #12205

mtreinish opened this issue Apr 18, 2024 · 1 comment · Fixed by #12459
Assignees
Labels
mod: circuit Related to the core of the `QuantumCircuit` class or the circuit library performance priority: high Rust This PR or issue is related to Rust code in the repository type: feature request New feature or request
Milestone

Comments

@mtreinish
Copy link
Member

What should we add?

As part of the ongoing effort to be able to more of Qiskit's operations in rust for performance and efficiency the last major piece (besides the DAGCircuit in #11721) are the operations object. This issue is to track the development of rust data structures that represent the the circuit's operations.

The main complexity here is that the python side of Qiskit exposes a very flexible and dynamic interface for defining operations. This includes fully custom operation objects that are defined in Python and generate their contents dynamically. We want to find a model where we retain this functionality and capability from Python as it's quite widely used, but also ensure we're storing an efficient representation in rust as the source of truth. The key goal of this work though is to enable working with gates from rust without any involvement from Python.

@mtreinish mtreinish added the type: feature request New feature or request label Apr 18, 2024
@mtreinish mtreinish added this to the 1.2.0 milestone Apr 18, 2024
@mtreinish
Copy link
Member Author

In my ideal model for the standard library our internal representation would be an enum that has a variant for each standard library gate and trait implementations for all of the data access. This would minimize what we store in the circuit structure to a minimal size. Something like:

#[derive(Clone)]
#[pyclass]
pub enum StandardGate {
    ZGate,
    YGate,
    XGate,
    CZGate,
    CYGate,
    CXGate,
    CCXGate,
    RXGate,
    RYGate,
    RZGate,
    ECRGate,
    SwapGate,
    SXGate,
    GlobalPhaseGate,
    IGate,
    HGate,
    ....
}

The trait impls for this enum aren't the most ergonomic to work with, but the efficiency of the structure seems worth it (especially as we don't modify the standard gate library too often)

@mtreinish mtreinish added priority: high mod: circuit Related to the core of the `QuantumCircuit` class or the circuit library Rust This PR or issue is related to Rust code in the repository labels Apr 18, 2024
mtreinish added a commit to mtreinish/qiskit-core that referenced this issue May 5, 2024
This compiles but has circular import errors because we need to
use the standard gate classes in Python to map from python to rust.

Fixes: Qiskit#12205
mtreinish added a commit to mtreinish/qiskit-core that referenced this issue May 5, 2024
This compiles but has circular import errors because we need to
use the standard gate classes in Python to map from python to rust.

Fixes: Qiskit#12205
mtreinish added a commit to mtreinish/qiskit-core that referenced this issue May 5, 2024
This compiles but has circular import errors because we need to
use the standard gate classes in Python to map from python to rust.

Fixes: Qiskit#12205
mtreinish added a commit to mtreinish/qiskit-core that referenced this issue May 6, 2024
This compiles but has circular import errors because we need to
use the standard gate classes in Python to map from python to rust.

Fixes: Qiskit#12205
mtreinish added a commit to mtreinish/qiskit-core that referenced this issue May 6, 2024
This compiles but has circular import errors because we need to
use the standard gate classes in Python to map from python to rust.

Fixes: Qiskit#12205
mtreinish added a commit to mtreinish/qiskit-core that referenced this issue May 7, 2024
This commit adds a native representation of Gates, Instruction, and
Operations to rust's circuit module. At a high level this works by
either wrapping the Python object in a rust wrapper struct that tracks
metadata about the operations (name, num_qubits, etc) and then for other
details it calls back to Python to get dynamic details like the
definition, matrix, etc. For standard library gates like Swap, CX, H,
etc this replaces the on circuit representation with a new enum
StandardGate. The enum representation is much more efficient and has a
minimal memory footprint as all the gate properties are defined in code
based on the enum variant (which represents the gate).

The use of an enum to represent standard gates does mean a change in
what we store on a CircuitInstruction. To represent a standard gate
fully we need to store the mutable properties of the existing Gate class
on the circuit instruction as the gate by itself doesn't contain this
detail. That means, the parameters, label, unit, duration, and condition
are added to the rust side of circuit instrucion. However no Python side
access methods are added for these as they're internal only to the rust
code and hopefully in Qiskit 2.0 we'll be able to drop, unit, duration,
and condition from the api leaving only label and parameters.

When an object is added to a CircuitInstruction the operation field is
translated to it's internal representation automatically. For standard
gates this translates it to the enum form, and for Python defined gates
this involves just wrapping them. Then whenever the operation field of
a circuit instruction object is read from python it converts it back to
the normal Python object representation.

This commit does not translate the full standard library of gates as
that would make the pull request huge, instead this adds the basic
infrastructure for having a more efficient standard gate representation
on circuits. There will be follow up pull requests to add the missing
gates and round out support in rust.

The goal of this pull request is primarily to add the infrastructure for
representing the full circuit model (and dag model in the future) in
rust. By itself this is not expected to improve runtime performance (if
anything it will probably hurt performance because of extra type
conversions) but it is intended to enable writing native circuit
manipulations in rust, including transpiler passes without needing
involvement from Python.

TODO:

- [ ] Fix parameter handling on copy (might involve moving parameter
  table to rust)
- [ ] Handle global phase in definitions (might mean moving a
  QuantumCircuit struct to rust instead of just CircuitData)
- [ ] Add definitions for gates migrated so far

Fixes: Qiskit#12205
mtreinish added a commit to mtreinish/qiskit-core that referenced this issue May 19, 2024
This commit adds a native representation of Gates, Instruction, and
Operations to rust's circuit module. At a high level this works by
either wrapping the Python object in a rust wrapper struct that tracks
metadata about the operations (name, num_qubits, etc) and then for other
details it calls back to Python to get dynamic details like the
definition, matrix, etc. For standard library gates like Swap, CX, H,
etc this replaces the on circuit representation with a new enum
StandardGate. The enum representation is much more efficient and has a
minimal memory footprint as all the gate properties are defined in code
based on the enum variant (which represents the gate).

The use of an enum to represent standard gates does mean a change in
what we store on a CircuitInstruction. To represent a standard gate
fully we need to store the mutable properties of the existing Gate class
on the circuit instruction as the gate by itself doesn't contain this
detail. That means, the parameters, label, unit, duration, and condition
are added to the rust side of circuit instrucion. However no Python side
access methods are added for these as they're internal only to the rust
code and hopefully in Qiskit 2.0 we'll be able to drop, unit, duration,
and condition from the api leaving only label and parameters.

When an object is added to a CircuitInstruction the operation field is
translated to it's internal representation automatically. For standard
gates this translates it to the enum form, and for Python defined gates
this involves just wrapping them. Then whenever the operation field of
a circuit instruction object is read from python it converts it back to
the normal Python object representation.

This commit does not translate the full standard library of gates as
that would make the pull request huge, instead this adds the basic
infrastructure for having a more efficient standard gate representation
on circuits. There will be follow up pull requests to add the missing
gates and round out support in rust.

The goal of this pull request is primarily to add the infrastructure for
representing the full circuit model (and dag model in the future) in
rust. By itself this is not expected to improve runtime performance (if
anything it will probably hurt performance because of extra type
conversions) but it is intended to enable writing native circuit
manipulations in rust, including transpiler passes without needing
involvement from Python.

TODO:

- [ ] Fix parameter handling on copy (might involve moving parameter
  table to rust)
- [ ] Handle global phase in definitions (might mean moving a
  QuantumCircuit struct to rust instead of just CircuitData)
- [ ] Add definitions for gates migrated so far

Fixes: Qiskit#12205
mtreinish added a commit to mtreinish/qiskit-core that referenced this issue May 25, 2024
This commit adds a native representation of Gates, Instruction, and
Operations to rust's circuit module. At a high level this works by
either wrapping the Python object in a rust wrapper struct that tracks
metadata about the operations (name, num_qubits, etc) and then for other
details it calls back to Python to get dynamic details like the
definition, matrix, etc. For standard library gates like Swap, CX, H,
etc this replaces the on-circuit representation with a new rust enum
StandardGate. The enum representation is much more efficient and has a
minimal memory footprint (just the enum variant and then any parameters
or other mutable state stored in the circuit instruction). All the gate
properties such as the matrix, definiton, name, etc are statically
defined in rust code based on the enum variant (which represents the
gate).

The use of an enum to represent standard gates does mean a change in
what we store on a CircuitInstruction. To represent a standard gate
fully we need to store the mutable properties of the existing Gate class
on the circuit instruction as the gate by itself doesn't contain this
detail. That means, the parameters, label, unit, duration, and condition
are added to the rust side of circuit instrucion. However no Python side
access methods are added for these as they're internal only to the Rust
code. In Qiskit 2.0 to simplify this storage we'll be able to drop, unit,
duration, and condition from the api leaving only label and parameters.
But for right now we're tracking all of the fields.

To facilitate working with circuits and gates full from rust the
setting the `operation` attribute of a `CircuitInstruction` object now
transltates the python object to an internal rust representation.
For standard gates this translates it to the enum form described earlier,
and for other circuit operations 3 new Rust structs: PyGate,
PyInstruction, and PyOperation are used to wrap the underlying Python
object in a Rust api. These structs cache some commonly accessed static
properties of the operation, such as the name, number of qubits, etc.
However for dynamic pieces, such as the definition or matrix, callback
to python to get a rust representation for those.

Similarly whenever the `operation` attribute is accessed from Python
it converts it back to the normal Python object representation. For
standard gates this involves creating a new instance of a Python object
based on it's internal rust representation. For the wrapper structs a
reference to the wrapped PyObject is returned.

To manage the 4 variants of operation (`StandardGate`, `PyGate`,
`PyInstruction`, and `PyOperation`) a new Rust trait `Operation` is
created that defines a standard interface for getting the properties
of a given circuit operation. This common interface is implemented for
the 4 variants as well as the `OperationType` enum which wraps all 4
(and is used as the type for `CircuitInstruction.operation` in the
rust code.

As everything in the `QuantumCircuit` data model is quite coupled moving
the source of truth for the operations to exist in Rust means that more
of the underlying `QuantumCircuit`'s responsibility has to move to Rust
as well. Primarily this involves the `ParameterTable` which was an
internal class for tracking which instructions in the circuit have a
`ParameterExpression` parameter so that when we go to bind parameters we
can lookup which operations need to be updated with the bind value.
Since the representation of those instructions now lives in Rust and
Python only recieves a ephemeral copy of the instructions the
ParameterTable had to be reimplemented in Rust to track the
instructions. This new parameter table maps the Parameter's uuid (as a
u128) as a unique identifier for each parameter and maps this to a
positional index in the circuit data to the underlying instruction using
that parameter. This is a bit different from the Python parameter table
which was mapping a parameter object to the id of the operation object
using that parmaeter. This also leads to a difference in the binding
mechanics as the parameter assignment was done by reference in the old
model, but now we need to update the entire instruction more explicitly
in rust. Additionally, because the global phase of a circuit can be
parameterized the ownership of global phase is moved from Python into
Rust in this commit as well.

After this commit the only properties of a circuit that are not defined
in Rust for the source of truth are the bits (and vars) of the circuit,
and when creating circuits from rust this is what causes a Python
interaction to still be required.

This commit does not translate the full standard library of gates as
that would make the pull request huge, instead this adds the basic
infrastructure for having a more efficient standard gate representation
on circuits. There will be follow up pull requests to add the missing
gates and round out support in rust.

The goal of this pull request is primarily to add the infrastructure for
representing the full circuit model (and dag model in the future) in
rust. By itself this is not expected to improve runtime performance (if
anything it will probably hurt performance because of extra type
conversions) but it is intended to enable writing native circuit
manipulations in Rust, including transpiler passes without needing
involvement from Python. Longer term this should greatly improve the
runtime performance and reduce the memory overhead of Qiskit. But,
this is just an early step towards that goal, and is more about
unlocking the future capability. The next steps after this commit are
to finish migrating the standard gate library and also update the
`QuantumCircuit` methods to better leverage the more complete rust
representation (which should help offset the performance penalty
introduced by this).

Fixes: Qiskit#12205
github-merge-queue bot pushed a commit that referenced this issue Jun 13, 2024
…2459)

* Add infrastructure for gates, instruction, and operations in Rust

This commit adds a native representation of Gates, Instruction, and
Operations to rust's circuit module. At a high level this works by
either wrapping the Python object in a rust wrapper struct that tracks
metadata about the operations (name, num_qubits, etc) and then for other
details it calls back to Python to get dynamic details like the
definition, matrix, etc. For standard library gates like Swap, CX, H,
etc this replaces the on-circuit representation with a new rust enum
StandardGate. The enum representation is much more efficient and has a
minimal memory footprint (just the enum variant and then any parameters
or other mutable state stored in the circuit instruction). All the gate
properties such as the matrix, definiton, name, etc are statically
defined in rust code based on the enum variant (which represents the
gate).

The use of an enum to represent standard gates does mean a change in
what we store on a CircuitInstruction. To represent a standard gate
fully we need to store the mutable properties of the existing Gate class
on the circuit instruction as the gate by itself doesn't contain this
detail. That means, the parameters, label, unit, duration, and condition
are added to the rust side of circuit instrucion. However no Python side
access methods are added for these as they're internal only to the Rust
code. In Qiskit 2.0 to simplify this storage we'll be able to drop, unit,
duration, and condition from the api leaving only label and parameters.
But for right now we're tracking all of the fields.

To facilitate working with circuits and gates full from rust the
setting the `operation` attribute of a `CircuitInstruction` object now
transltates the python object to an internal rust representation.
For standard gates this translates it to the enum form described earlier,
and for other circuit operations 3 new Rust structs: PyGate,
PyInstruction, and PyOperation are used to wrap the underlying Python
object in a Rust api. These structs cache some commonly accessed static
properties of the operation, such as the name, number of qubits, etc.
However for dynamic pieces, such as the definition or matrix, callback
to python to get a rust representation for those.

Similarly whenever the `operation` attribute is accessed from Python
it converts it back to the normal Python object representation. For
standard gates this involves creating a new instance of a Python object
based on it's internal rust representation. For the wrapper structs a
reference to the wrapped PyObject is returned.

To manage the 4 variants of operation (`StandardGate`, `PyGate`,
`PyInstruction`, and `PyOperation`) a new Rust trait `Operation` is
created that defines a standard interface for getting the properties
of a given circuit operation. This common interface is implemented for
the 4 variants as well as the `OperationType` enum which wraps all 4
(and is used as the type for `CircuitInstruction.operation` in the
rust code.

As everything in the `QuantumCircuit` data model is quite coupled moving
the source of truth for the operations to exist in Rust means that more
of the underlying `QuantumCircuit`'s responsibility has to move to Rust
as well. Primarily this involves the `ParameterTable` which was an
internal class for tracking which instructions in the circuit have a
`ParameterExpression` parameter so that when we go to bind parameters we
can lookup which operations need to be updated with the bind value.
Since the representation of those instructions now lives in Rust and
Python only recieves a ephemeral copy of the instructions the
ParameterTable had to be reimplemented in Rust to track the
instructions. This new parameter table maps the Parameter's uuid (as a
u128) as a unique identifier for each parameter and maps this to a
positional index in the circuit data to the underlying instruction using
that parameter. This is a bit different from the Python parameter table
which was mapping a parameter object to the id of the operation object
using that parmaeter. This also leads to a difference in the binding
mechanics as the parameter assignment was done by reference in the old
model, but now we need to update the entire instruction more explicitly
in rust. Additionally, because the global phase of a circuit can be
parameterized the ownership of global phase is moved from Python into
Rust in this commit as well.

After this commit the only properties of a circuit that are not defined
in Rust for the source of truth are the bits (and vars) of the circuit,
and when creating circuits from rust this is what causes a Python
interaction to still be required.

This commit does not translate the full standard library of gates as
that would make the pull request huge, instead this adds the basic
infrastructure for having a more efficient standard gate representation
on circuits. There will be follow up pull requests to add the missing
gates and round out support in rust.

The goal of this pull request is primarily to add the infrastructure for
representing the full circuit model (and dag model in the future) in
rust. By itself this is not expected to improve runtime performance (if
anything it will probably hurt performance because of extra type
conversions) but it is intended to enable writing native circuit
manipulations in Rust, including transpiler passes without needing
involvement from Python. Longer term this should greatly improve the
runtime performance and reduce the memory overhead of Qiskit. But,
this is just an early step towards that goal, and is more about
unlocking the future capability. The next steps after this commit are
to finish migrating the standard gate library and also update the
`QuantumCircuit` methods to better leverage the more complete rust
representation (which should help offset the performance penalty
introduced by this).

Fixes: #12205

* Fix Python->Rust Param conversion

This commit adds a custom implementation of the FromPyObject trait for
the Param enum. Previously, the Param trait derived it's impl of the
trait, but this logic wasn't perfect. In cases whern a
ParameterExpression was effectively a constant (such as `0 * x`) the
trait's attempt to coerce to a float first would result in those
ParameterExpressions being dropped from the circuit at insertion time.
This was a change in behavior from before having gates in Rust as the
parameters would disappear from the circuit at insertion time instead of
at bind time. This commit fixes this by having a custom impl for
FromPyObject that first tries to figure out if the parameter is a
ParameterExpression (or a QuantumCircuit) by using a Python isinstance()
check, then tries to extract it as a float, and finally stores a
non-parameter object; which is a new variant in the Param enum. This
new variant also lets us simplify the logic around adding gates to the
parameter table as we're able to know ahead of time which gate
parameters are `ParameterExpression`s and which are other objects (and
don't need to be tracked in the parameter table.

Additionally this commit tweaks two tests, the first is
test.python.circuit.library.test_nlocal.TestNLocal.test_parameters_setter
which was adjusted in the previous commit to workaround the bug fixed
by this commit. The second is test.python.circuit.test_parameters which
was testing that a bound ParameterExpression with a value of 0 defaults
to an int which was a side effect of passing an int input to symengine
for the bind value and not part of the api and didn't need to be
checked. This assertion was removed from the test because the rust
representation is only storing f64 values for the numeric parameters
and it is never an int after binding from the Python perspective it
isn't any different to have float(0) and int(0) unless you explicit
isinstance check like the test previously was.

* Fix qasm3 exporter for std gates without stdgates.inc

This commit fixes the handling of standard gates in Qiskit when the user
specifies excluding the use of the stdgates.inc file from the exported
qasm. Previously the object id of the standard gates were used to
maintain a lookup table of the global definitions for all the standard
gates explicitly in the file. However, the rust refactor means that
every time the exporter accesses `circuit.data[x].operation` a new
instance is returned. This means that on subsequent lookups for the
definition the gate definitions are never found. To correct this issue
this commit adds to the lookup table a fallback of the gate name +
parameters to do the lookup for. This should be unique for any standard
gate and not interfere with the previous logic that's still in place and
functional for other custom gate definitions.

While this fixes the logic in the exporter the test is still failing
because the test is asserting the object ids are the same in the qasm3
file, which isn't the case anymore. The test will be updated in a
subsequent commit to validate the qasm3 file is correct without using
a hardcoded object id.

* Fix base scheduler analysis pass duration setting

When ALAPScheduleAnalysis and ASAPScheduleAnalysis were setting the
duration of a gate they were doing `node.op.duration = duration` this
wasn't always working because if `node.op` was a standard gate it
returned a new Python object created from the underlying rust
representation. This commit fixes the passes so that they modify the
duration and then explicit set the operation to update it's rust
representation.

* Fix python lint

* Fix last failing qasm3 test for std gates without stdgates.inc

While the logic for the qasm3 exporter was fixed
in commit a6e69ba to handle the edge
case of a user specifying that the qasm exporter does not use the
stdgates.inc include file in the output, but also has qiskit's standard
gates in their circuit being exported. The one unit test to provide
coverage for that scenario was not passing because when an id was used
for the gate definitions in the qasm3 file it was being referenced
against a temporary created by accessing a standard gate from the
circuit and the ids weren't the same so the reference string didn't
match what the exporter generated. This commit fixes this by changing
the test to not do an exact string comparison, but instead a line by
line comparison that either does exact equality check or a regex search
for the expected line and the ids are checked as being any 15 character
integer.

* Remove superfluous comment

* Cache imported classes with GILOnceCell

* Remove unused python variables

* Add missing file

* Update QuantumCircuit gate methods to bypass Python object

This commit updates the QuantumCircuit gate methods which add a given
gate to the circuit to bypass the python gate object creation and
directly insert a rust representation of the gate. This avoids a
conversion in the rust side of the code. While in practice this is just
the Python side object creation and a getattr for the rust code to
determine it's a standard gate that we're skipping. This may add up over
time if there are a lot of gates being created by the method.

To accomplish this the rust code handling the mapping of rust
StandardGate variants to the Python classes that represent those gates
needed to be updated as well. By bypassing the python object creation
we need a fallback to populate the gate class for when a user access the
operation object from Python. Previously this mapping was only being
populated at insertion time and if we never insert the python object
(for a circuit created only via the methods) then we need a way to find
what the gate class is. A static lookup table of import paths and class names
are added to `qiskit_circuit::imports` module to faciliate this and
helper functions are added to facilitate interacting with the class
objects that represent each gate.

* Deduplicate gate matrix definitions

* Fix lint

* Attempt to fix qasm3 test failure

* Add compile time option to cache py gate returns for rust std gates

This commit adds a new rust crate feature flag for the qiskit-circuits
and qiskit-pyext that enables caching the output from
CircuitInstruction.operation to python space. Previously, for memory
efficiency we were reconstructing the python object on demand for every
access. This was to avoid carrying around an extra pointer and keeping
the ephemeral python object around longer term if it's only needed once.
But right now nothing is directly using the rust representation yet and
everything is accessing via the python interface, so recreating gate
objects on the fly has a huge performance penalty. To avoid that this
adds caching by default as a temporary solution to avoid this until we
have more usage of the rust representation of gates.

There is an inherent tension between an optimal rust representation
and something that is performant for Python access and there isn't a
clear cut answer on which one is better to optimize for. A build time
feature lets the user pick, if what we settle on for the default doesn't
agree with their priorities or use case. Personally I'd like to see us
disable the caching longer term (hopefully before releasing this
functionality), but that's dependent on a sufficent level of usage from
rust superseding the current Python space usage in the core of Qiskit.

* Add num_nonlocal_gates implementation in rust

This commit adds a native rust implementation to rust for the
num_nonlocal_gates method on QuantumCircuit. Now that we have a rust
representation of gates it is potentially faster to do the count because
the iteration and filtering is done rust side.

* Performance tuning circuit construction

This commit fixes some performance issues with the addition of standard
gates to a circuit. To workaround potential reference cycles in Python
when calling rust we need to check the parameters of the operation. This
was causing our fast path for standard gates to access the `operation`
attribute to get the parameters. This causes the gate to be eagerly
constructed on the getter. However, the reference cycle case can only
happen in situations without a standard gate, and the fast path for
adding standard gates directly won't need to run this so a skip is added
if we're adding a standard gate.

* Add back validation of parameters on gate methods

In the previous commit a side effect of the accidental eager operation
creation was that the parameter input for gates were being validated by
that. By fixing that in the previous commit the validation of input
parameters on the circuit methods was broken. This commit fixes that
oversight and adds back the validation.

* Skip validation on gate creation from rust

* Offload operation copying to rust

This commit fixes a performance regression in the
`QuantumCircuit.copy()` method which was previously using Python to copy
the operations which had extra overhead to go from rust to python and
vice versa. This moves that logic to exist in rust and improve the copy
performance.

* Fix lint

* Perform deepcopy in rust

This commit moves the deepcopy handling to occur solely in Rust.
Previously each instruction would be directly deepcopied by iterating
over the circuit data. However, we can do this rust side now and doing
this is more efficient because while we need to rely on Python to run a
deepcopy we can skip it for the Rust standard gates and rely on Rust to
copy those gates.

* Fix QuantumCircuit.compose() performance regression

This commit fixes a performance regression in the compose() method. This
was caused by the checking for classical conditions in the method
requiring eagerly converting all standard gates to a Python object. This
changes the logic to do this only if we know we have a condition (which
we can determine Python side now).

* Fix map_ops test case with no caching case

* Fix typos in docs

This commit fixes several docs typos that were caught during code review.

Co-authored-by: Eli Arbel <[email protected]>

* Shrink memory usage for extra mutable instruction state

This commit changes how we store the extra mutable instruction state
(condition, duration, unit, and label) for each `CircuitInstruction`
and `PackedInstruction` in the circuit. Previously it was all stored
as separate `Option<T>` fields on the struct, which required at least
a pointer's width for each field which was wasted space the majority of
the time as using these fields are not common. To optimize the memory
layout of the struct this moves these attributes to a new struct which
is put in an `Option<Box<_>>` which reduces it from 4 pointer widths
down to 1 per object. This comes from extra runtime cost from the extra
layer of pointer indirection but as this is the uncommon path this
tradeoff is fine.

* Remove Option<> from params field in CircuitInstruction

This commit removes the Option<> from the params field in
CircuitInstruction. There is no real distinction between an empty vec
and None in this case, so the option just added another layer in the API
that we didn't need to deal with. Also depending on the memory alignment
using an Option<T> might have ended up in a little extra memory usage
too, so removing it removes that potential source of overhead.

* Eagerly construct rust python wrappers in .append()

This commit updates the Python code in QuantumCircuit.append() method
to eagerly construct the rust wrapper objects for python defined circuit
operations.

* Simplify code around handling python errors in rust

* Revert "Skip validation on gate creation from rust"

This reverts commit 2f81bde. The
validation skipping was unsound in some cases and could lead to invalid
circuit being generated. If we end up needing this as an optimization we
can remove this in the future in a follow-up PR that explores this in
isolation.

* Temporarily use git for qasm3 import

In Qiskit/qiskit-qasm3-import#34 the issue we're hitting caused by
qiskit-qasm3-import using the private circuit attributes removed in this
PR was fixed. This commit temporarily moves to installing it from git so
we can fully run CI. When qiskit-qasm3-import is released we should
revert this commit.

* Fix lint

* Fix lint for real (we really need to use a py312 compatible version of pylint)

* Fix test failure caused by incorrect lint fix

* Relax trait-method typing requirements

* Encapsulate `GILOnceCell` initialisers to local logic

* Simplify Interface for building circuit of standard gates in rust

* Simplify complex64 creation in gate_matrix.rs

This just switches Complex64::new(re, im) to be c64(re, im) to reduce
the amount of typing. c64 needs to be defined inplace so it can be a
const fn.

* Simplify initialization of array of elements that are not Copy (#28)

* Simplify initialization of array of elements that are not Copy

* Only generate array when necessary

* Fix doc typos

Co-authored-by: Kevin Hartman <[email protected]>

* Add conversion trait for OperationType -> OperationInput and simplify CircuitInstruction::replace()

* Use destructuring for operation_type_to_py extra attr handling

* Simplify trait bounds for map_indices()

The map_indices() method previously specified both Iterator and
ExactSizeIterator for it's trait bounds, but Iterator is a supertrait of
ExactSizeIterator and we don't need to explicitly list both. This commit
removes the duplicate trait bound.

* Make Qubit and Clbit newtype member public

As we start to use Qubit and Clbit for creating circuits from accelerate
and other crates in the Qiskit workspace we need to be able to create
instances of them. However, the newtype member BitType was not public
which prevented creating new Qubits. This commit fixes this by making it
public.

* Use snakecase for gate matrix names

* Remove pointless underscore prefix

* Use downcast instead of bound

* Rwork _append reference cycle handling

This commit reworks the multiple borrow handling in the _append() method
to leveraging `Bound.try_borrow()` to return a consistent error message
if we're unable to borrow a CircuitInstruction in the rust code meaning
there is a cyclical reference in the code. Previously we tried to detect
this cycle up-front which added significant overhead for a corner case.

* Make CircuitData.global_phase_param_index a class attr

* Use &[Param] instead of &SmallVec<..> for operation_type_and_data_to_py

* Have get_params_unsorted return a set

* Use lookup table for static property methods of StandardGate

* Use PyTuple::empty_bound()

* Fix lint

* Add missing test method docstring

* Reuse allocations in parameter table update

* Remove unnecessary global phase zeroing

* Move manually set params to a separate function

* Fix release note typo

* Use constant for global-phase index

* Switch requirement to release version

---------

Co-authored-by: Eli Arbel <[email protected]>
Co-authored-by: Jake Lishman <[email protected]>
Co-authored-by: John Lapeyre <[email protected]>
Co-authored-by: Kevin Hartman <[email protected]>
Procatv pushed a commit to Procatv/qiskit-terra-catherines that referenced this issue Aug 1, 2024
…skit#12459)

* Add infrastructure for gates, instruction, and operations in Rust

This commit adds a native representation of Gates, Instruction, and
Operations to rust's circuit module. At a high level this works by
either wrapping the Python object in a rust wrapper struct that tracks
metadata about the operations (name, num_qubits, etc) and then for other
details it calls back to Python to get dynamic details like the
definition, matrix, etc. For standard library gates like Swap, CX, H,
etc this replaces the on-circuit representation with a new rust enum
StandardGate. The enum representation is much more efficient and has a
minimal memory footprint (just the enum variant and then any parameters
or other mutable state stored in the circuit instruction). All the gate
properties such as the matrix, definiton, name, etc are statically
defined in rust code based on the enum variant (which represents the
gate).

The use of an enum to represent standard gates does mean a change in
what we store on a CircuitInstruction. To represent a standard gate
fully we need to store the mutable properties of the existing Gate class
on the circuit instruction as the gate by itself doesn't contain this
detail. That means, the parameters, label, unit, duration, and condition
are added to the rust side of circuit instrucion. However no Python side
access methods are added for these as they're internal only to the Rust
code. In Qiskit 2.0 to simplify this storage we'll be able to drop, unit,
duration, and condition from the api leaving only label and parameters.
But for right now we're tracking all of the fields.

To facilitate working with circuits and gates full from rust the
setting the `operation` attribute of a `CircuitInstruction` object now
transltates the python object to an internal rust representation.
For standard gates this translates it to the enum form described earlier,
and for other circuit operations 3 new Rust structs: PyGate,
PyInstruction, and PyOperation are used to wrap the underlying Python
object in a Rust api. These structs cache some commonly accessed static
properties of the operation, such as the name, number of qubits, etc.
However for dynamic pieces, such as the definition or matrix, callback
to python to get a rust representation for those.

Similarly whenever the `operation` attribute is accessed from Python
it converts it back to the normal Python object representation. For
standard gates this involves creating a new instance of a Python object
based on it's internal rust representation. For the wrapper structs a
reference to the wrapped PyObject is returned.

To manage the 4 variants of operation (`StandardGate`, `PyGate`,
`PyInstruction`, and `PyOperation`) a new Rust trait `Operation` is
created that defines a standard interface for getting the properties
of a given circuit operation. This common interface is implemented for
the 4 variants as well as the `OperationType` enum which wraps all 4
(and is used as the type for `CircuitInstruction.operation` in the
rust code.

As everything in the `QuantumCircuit` data model is quite coupled moving
the source of truth for the operations to exist in Rust means that more
of the underlying `QuantumCircuit`'s responsibility has to move to Rust
as well. Primarily this involves the `ParameterTable` which was an
internal class for tracking which instructions in the circuit have a
`ParameterExpression` parameter so that when we go to bind parameters we
can lookup which operations need to be updated with the bind value.
Since the representation of those instructions now lives in Rust and
Python only recieves a ephemeral copy of the instructions the
ParameterTable had to be reimplemented in Rust to track the
instructions. This new parameter table maps the Parameter's uuid (as a
u128) as a unique identifier for each parameter and maps this to a
positional index in the circuit data to the underlying instruction using
that parameter. This is a bit different from the Python parameter table
which was mapping a parameter object to the id of the operation object
using that parmaeter. This also leads to a difference in the binding
mechanics as the parameter assignment was done by reference in the old
model, but now we need to update the entire instruction more explicitly
in rust. Additionally, because the global phase of a circuit can be
parameterized the ownership of global phase is moved from Python into
Rust in this commit as well.

After this commit the only properties of a circuit that are not defined
in Rust for the source of truth are the bits (and vars) of the circuit,
and when creating circuits from rust this is what causes a Python
interaction to still be required.

This commit does not translate the full standard library of gates as
that would make the pull request huge, instead this adds the basic
infrastructure for having a more efficient standard gate representation
on circuits. There will be follow up pull requests to add the missing
gates and round out support in rust.

The goal of this pull request is primarily to add the infrastructure for
representing the full circuit model (and dag model in the future) in
rust. By itself this is not expected to improve runtime performance (if
anything it will probably hurt performance because of extra type
conversions) but it is intended to enable writing native circuit
manipulations in Rust, including transpiler passes without needing
involvement from Python. Longer term this should greatly improve the
runtime performance and reduce the memory overhead of Qiskit. But,
this is just an early step towards that goal, and is more about
unlocking the future capability. The next steps after this commit are
to finish migrating the standard gate library and also update the
`QuantumCircuit` methods to better leverage the more complete rust
representation (which should help offset the performance penalty
introduced by this).

Fixes: Qiskit#12205

* Fix Python->Rust Param conversion

This commit adds a custom implementation of the FromPyObject trait for
the Param enum. Previously, the Param trait derived it's impl of the
trait, but this logic wasn't perfect. In cases whern a
ParameterExpression was effectively a constant (such as `0 * x`) the
trait's attempt to coerce to a float first would result in those
ParameterExpressions being dropped from the circuit at insertion time.
This was a change in behavior from before having gates in Rust as the
parameters would disappear from the circuit at insertion time instead of
at bind time. This commit fixes this by having a custom impl for
FromPyObject that first tries to figure out if the parameter is a
ParameterExpression (or a QuantumCircuit) by using a Python isinstance()
check, then tries to extract it as a float, and finally stores a
non-parameter object; which is a new variant in the Param enum. This
new variant also lets us simplify the logic around adding gates to the
parameter table as we're able to know ahead of time which gate
parameters are `ParameterExpression`s and which are other objects (and
don't need to be tracked in the parameter table.

Additionally this commit tweaks two tests, the first is
test.python.circuit.library.test_nlocal.TestNLocal.test_parameters_setter
which was adjusted in the previous commit to workaround the bug fixed
by this commit. The second is test.python.circuit.test_parameters which
was testing that a bound ParameterExpression with a value of 0 defaults
to an int which was a side effect of passing an int input to symengine
for the bind value and not part of the api and didn't need to be
checked. This assertion was removed from the test because the rust
representation is only storing f64 values for the numeric parameters
and it is never an int after binding from the Python perspective it
isn't any different to have float(0) and int(0) unless you explicit
isinstance check like the test previously was.

* Fix qasm3 exporter for std gates without stdgates.inc

This commit fixes the handling of standard gates in Qiskit when the user
specifies excluding the use of the stdgates.inc file from the exported
qasm. Previously the object id of the standard gates were used to
maintain a lookup table of the global definitions for all the standard
gates explicitly in the file. However, the rust refactor means that
every time the exporter accesses `circuit.data[x].operation` a new
instance is returned. This means that on subsequent lookups for the
definition the gate definitions are never found. To correct this issue
this commit adds to the lookup table a fallback of the gate name +
parameters to do the lookup for. This should be unique for any standard
gate and not interfere with the previous logic that's still in place and
functional for other custom gate definitions.

While this fixes the logic in the exporter the test is still failing
because the test is asserting the object ids are the same in the qasm3
file, which isn't the case anymore. The test will be updated in a
subsequent commit to validate the qasm3 file is correct without using
a hardcoded object id.

* Fix base scheduler analysis pass duration setting

When ALAPScheduleAnalysis and ASAPScheduleAnalysis were setting the
duration of a gate they were doing `node.op.duration = duration` this
wasn't always working because if `node.op` was a standard gate it
returned a new Python object created from the underlying rust
representation. This commit fixes the passes so that they modify the
duration and then explicit set the operation to update it's rust
representation.

* Fix python lint

* Fix last failing qasm3 test for std gates without stdgates.inc

While the logic for the qasm3 exporter was fixed
in commit a6e69ba to handle the edge
case of a user specifying that the qasm exporter does not use the
stdgates.inc include file in the output, but also has qiskit's standard
gates in their circuit being exported. The one unit test to provide
coverage for that scenario was not passing because when an id was used
for the gate definitions in the qasm3 file it was being referenced
against a temporary created by accessing a standard gate from the
circuit and the ids weren't the same so the reference string didn't
match what the exporter generated. This commit fixes this by changing
the test to not do an exact string comparison, but instead a line by
line comparison that either does exact equality check or a regex search
for the expected line and the ids are checked as being any 15 character
integer.

* Remove superfluous comment

* Cache imported classes with GILOnceCell

* Remove unused python variables

* Add missing file

* Update QuantumCircuit gate methods to bypass Python object

This commit updates the QuantumCircuit gate methods which add a given
gate to the circuit to bypass the python gate object creation and
directly insert a rust representation of the gate. This avoids a
conversion in the rust side of the code. While in practice this is just
the Python side object creation and a getattr for the rust code to
determine it's a standard gate that we're skipping. This may add up over
time if there are a lot of gates being created by the method.

To accomplish this the rust code handling the mapping of rust
StandardGate variants to the Python classes that represent those gates
needed to be updated as well. By bypassing the python object creation
we need a fallback to populate the gate class for when a user access the
operation object from Python. Previously this mapping was only being
populated at insertion time and if we never insert the python object
(for a circuit created only via the methods) then we need a way to find
what the gate class is. A static lookup table of import paths and class names
are added to `qiskit_circuit::imports` module to faciliate this and
helper functions are added to facilitate interacting with the class
objects that represent each gate.

* Deduplicate gate matrix definitions

* Fix lint

* Attempt to fix qasm3 test failure

* Add compile time option to cache py gate returns for rust std gates

This commit adds a new rust crate feature flag for the qiskit-circuits
and qiskit-pyext that enables caching the output from
CircuitInstruction.operation to python space. Previously, for memory
efficiency we were reconstructing the python object on demand for every
access. This was to avoid carrying around an extra pointer and keeping
the ephemeral python object around longer term if it's only needed once.
But right now nothing is directly using the rust representation yet and
everything is accessing via the python interface, so recreating gate
objects on the fly has a huge performance penalty. To avoid that this
adds caching by default as a temporary solution to avoid this until we
have more usage of the rust representation of gates.

There is an inherent tension between an optimal rust representation
and something that is performant for Python access and there isn't a
clear cut answer on which one is better to optimize for. A build time
feature lets the user pick, if what we settle on for the default doesn't
agree with their priorities or use case. Personally I'd like to see us
disable the caching longer term (hopefully before releasing this
functionality), but that's dependent on a sufficent level of usage from
rust superseding the current Python space usage in the core of Qiskit.

* Add num_nonlocal_gates implementation in rust

This commit adds a native rust implementation to rust for the
num_nonlocal_gates method on QuantumCircuit. Now that we have a rust
representation of gates it is potentially faster to do the count because
the iteration and filtering is done rust side.

* Performance tuning circuit construction

This commit fixes some performance issues with the addition of standard
gates to a circuit. To workaround potential reference cycles in Python
when calling rust we need to check the parameters of the operation. This
was causing our fast path for standard gates to access the `operation`
attribute to get the parameters. This causes the gate to be eagerly
constructed on the getter. However, the reference cycle case can only
happen in situations without a standard gate, and the fast path for
adding standard gates directly won't need to run this so a skip is added
if we're adding a standard gate.

* Add back validation of parameters on gate methods

In the previous commit a side effect of the accidental eager operation
creation was that the parameter input for gates were being validated by
that. By fixing that in the previous commit the validation of input
parameters on the circuit methods was broken. This commit fixes that
oversight and adds back the validation.

* Skip validation on gate creation from rust

* Offload operation copying to rust

This commit fixes a performance regression in the
`QuantumCircuit.copy()` method which was previously using Python to copy
the operations which had extra overhead to go from rust to python and
vice versa. This moves that logic to exist in rust and improve the copy
performance.

* Fix lint

* Perform deepcopy in rust

This commit moves the deepcopy handling to occur solely in Rust.
Previously each instruction would be directly deepcopied by iterating
over the circuit data. However, we can do this rust side now and doing
this is more efficient because while we need to rely on Python to run a
deepcopy we can skip it for the Rust standard gates and rely on Rust to
copy those gates.

* Fix QuantumCircuit.compose() performance regression

This commit fixes a performance regression in the compose() method. This
was caused by the checking for classical conditions in the method
requiring eagerly converting all standard gates to a Python object. This
changes the logic to do this only if we know we have a condition (which
we can determine Python side now).

* Fix map_ops test case with no caching case

* Fix typos in docs

This commit fixes several docs typos that were caught during code review.

Co-authored-by: Eli Arbel <[email protected]>

* Shrink memory usage for extra mutable instruction state

This commit changes how we store the extra mutable instruction state
(condition, duration, unit, and label) for each `CircuitInstruction`
and `PackedInstruction` in the circuit. Previously it was all stored
as separate `Option<T>` fields on the struct, which required at least
a pointer's width for each field which was wasted space the majority of
the time as using these fields are not common. To optimize the memory
layout of the struct this moves these attributes to a new struct which
is put in an `Option<Box<_>>` which reduces it from 4 pointer widths
down to 1 per object. This comes from extra runtime cost from the extra
layer of pointer indirection but as this is the uncommon path this
tradeoff is fine.

* Remove Option<> from params field in CircuitInstruction

This commit removes the Option<> from the params field in
CircuitInstruction. There is no real distinction between an empty vec
and None in this case, so the option just added another layer in the API
that we didn't need to deal with. Also depending on the memory alignment
using an Option<T> might have ended up in a little extra memory usage
too, so removing it removes that potential source of overhead.

* Eagerly construct rust python wrappers in .append()

This commit updates the Python code in QuantumCircuit.append() method
to eagerly construct the rust wrapper objects for python defined circuit
operations.

* Simplify code around handling python errors in rust

* Revert "Skip validation on gate creation from rust"

This reverts commit 2f81bde. The
validation skipping was unsound in some cases and could lead to invalid
circuit being generated. If we end up needing this as an optimization we
can remove this in the future in a follow-up PR that explores this in
isolation.

* Temporarily use git for qasm3 import

In Qiskit/qiskit-qasm3-import#34 the issue we're hitting caused by
qiskit-qasm3-import using the private circuit attributes removed in this
PR was fixed. This commit temporarily moves to installing it from git so
we can fully run CI. When qiskit-qasm3-import is released we should
revert this commit.

* Fix lint

* Fix lint for real (we really need to use a py312 compatible version of pylint)

* Fix test failure caused by incorrect lint fix

* Relax trait-method typing requirements

* Encapsulate `GILOnceCell` initialisers to local logic

* Simplify Interface for building circuit of standard gates in rust

* Simplify complex64 creation in gate_matrix.rs

This just switches Complex64::new(re, im) to be c64(re, im) to reduce
the amount of typing. c64 needs to be defined inplace so it can be a
const fn.

* Simplify initialization of array of elements that are not Copy (Qiskit#28)

* Simplify initialization of array of elements that are not Copy

* Only generate array when necessary

* Fix doc typos

Co-authored-by: Kevin Hartman <[email protected]>

* Add conversion trait for OperationType -> OperationInput and simplify CircuitInstruction::replace()

* Use destructuring for operation_type_to_py extra attr handling

* Simplify trait bounds for map_indices()

The map_indices() method previously specified both Iterator and
ExactSizeIterator for it's trait bounds, but Iterator is a supertrait of
ExactSizeIterator and we don't need to explicitly list both. This commit
removes the duplicate trait bound.

* Make Qubit and Clbit newtype member public

As we start to use Qubit and Clbit for creating circuits from accelerate
and other crates in the Qiskit workspace we need to be able to create
instances of them. However, the newtype member BitType was not public
which prevented creating new Qubits. This commit fixes this by making it
public.

* Use snakecase for gate matrix names

* Remove pointless underscore prefix

* Use downcast instead of bound

* Rwork _append reference cycle handling

This commit reworks the multiple borrow handling in the _append() method
to leveraging `Bound.try_borrow()` to return a consistent error message
if we're unable to borrow a CircuitInstruction in the rust code meaning
there is a cyclical reference in the code. Previously we tried to detect
this cycle up-front which added significant overhead for a corner case.

* Make CircuitData.global_phase_param_index a class attr

* Use &[Param] instead of &SmallVec<..> for operation_type_and_data_to_py

* Have get_params_unsorted return a set

* Use lookup table for static property methods of StandardGate

* Use PyTuple::empty_bound()

* Fix lint

* Add missing test method docstring

* Reuse allocations in parameter table update

* Remove unnecessary global phase zeroing

* Move manually set params to a separate function

* Fix release note typo

* Use constant for global-phase index

* Switch requirement to release version

---------

Co-authored-by: Eli Arbel <[email protected]>
Co-authored-by: Jake Lishman <[email protected]>
Co-authored-by: John Lapeyre <[email protected]>
Co-authored-by: Kevin Hartman <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mod: circuit Related to the core of the `QuantumCircuit` class or the circuit library performance priority: high Rust This PR or issue is related to Rust code in the repository type: feature request New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants