-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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 infrastructure for gates, instruction, and operations in Rust #12459
Conversation
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
One or more of the following people are relevant to this code:
|
Oh, those neko failures are interesting and unexpected (I was expecting the 6 unit test failures in terra's unit tests). Looking at it I think this is the same case of something I hit in the nlocal parameter tests. The underlying issue is that for the type of the elements of param vector in rust I defined basically the following type: #[derive(FromPyObject)]
enum Param {
Float(f64),
ParameterExpression(PyObject)
} and the issue is the derive p = Parameter('p')
expression = p * 0 and then uses |
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.
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.
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.
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.
Pull Request Test Coverage Report for Build 9496969108Warning: 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.
Details
💛 - Coveralls |
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.
Since Qiskit#12459 recently merged we now have a mechanism to build a circuit from rust. This commit updates the synthesis function to build the circuit directly in rust instead of returning a circuit sequence and building the circuit from Python. This should speed up the construction substantially.
- Create custom enum to collect either a `NormalOperation` or a `VariableOperation` depending on what is needed. - Add a rust native `is_instruction_supported` method to check whether a Target supports a certain instruction. - Make conversion methods from `circuit_instruction.rs` public. - Add comparison methods for `Param` in `operations.rs` - Remove need for isclass method in rustwise `add_instruction` - Other tweaks and fixes.
This PR introduces some abbreviations for repetitive Rust code. Motivations are reducing clutter, improving readability, and perhaps modest support for rapid development. * Use the definition of `const fn 64` that was introduced in Qiskit#12459 uniformly in all crates. * Define some complex constants `C_ONE`, `C_ZERO`, `IM`, etc. * Introduce type definitions for arrays representing gates. For example: `GateArray1Q = [[Complex64; 2]; 2];`
This PR introduces some abbreviations for repetitive Rust code. Motivations are reducing clutter, improving readability, and perhaps modest support for rapid development. * Use the definition of `const fn 64` that was introduced in #12459 uniformly in all crates. * Define some complex constants `C_ONE`, `C_ZERO`, `IM`, etc. * Introduce type definitions for arrays representing gates. For example: `GateArray1Q = [[Complex64; 2]; 2];`
Since Qiskit#12459 accessing `node.op` in the transpiler eagerly creates a Python object on access. This is because we now are no longer storing a Python object internally and we need to rebuild the object to return the python object as expected by the api. This is causing a significant performance regression because of the extra overhead. The longer term goal is to move as much of the performance critical passes to operate in rust which will eliminate this overhead. But in the meantime we can mitigate the performance overhead by changing the Python access patterns to avoid the operation object creation. This commit adds some new getter methods to DAGOpNode to give access to the inner rust data so that we can avoid the extra overhead. As a proof of concept this updates the unitary synthesis pass in isolation. Doing this fixes the regression caused by Qiskit#12459 for that pass. We can continue this migration for everything else in follow up PRs. This commit is mostly to establish the pattern and add the python space access methods.
This commit updates the commutative cancellation and commutation analysis transpiler pass. It builds off of Qiskit#12692 to adjust access patterns in the python transpiler path to avoid eagerly creating a Python space operation object. The goal of this PR is to mitigate the performance regression on these passes introduced by the extra conversion cost of Qiskit#12459. As part of this the commutation checker is rewritten in rust since all that requires is gates in rust which we've had a representation of since Qiskit#12459 merged.
This commit updates the commutative cancellation and commutation analysis transpiler pass. It builds off of Qiskit#12692 to adjust access patterns in the python transpiler path to avoid eagerly creating a Python space operation object. The goal of this PR is to mitigate the performance regression on these passes introduced by the extra conversion cost of Qiskit#12459.
This commit moves to use rust gates for the ConsolidateBlocks transpiler pass. Instead of generating the unitary matrices for the gates in a 2q block Python side and passing that list to a rust function this commit switches to passing a list of DAGOpNodes to the rust and then generating the matrices inside the rust function directly. This is similar to what was done in Qiskit#12650 for Optimize1qGatesDecomposition. Besides being faster to get the matrix for standard gates, it also reduces the eager construction of Python gate objects which was a significant source of overhead after Qiskit#12459. To that end this builds on the thread of work in the two PRs Qiskit#12692 and Qiskit#12701 which changed the access patterns for other passes to minimize eager gate object construction.
This commit updates the BasisTranslator transpiler pass. It builds off of Qiskit#12692 and Qiskit#12701 to adjust access patterns in the python transpiler path to avoid eagerly creating a Python space operation object. The goal of this PR is to mitigate the performance regression introduced by the extra conversion cost of Qiskit#12459 on the BasisTranslator.
* Avoid Python operation creation in transpiler Since #12459 accessing `node.op` in the transpiler eagerly creates a Python object on access. This is because we now are no longer storing a Python object internally and we need to rebuild the object to return the python object as expected by the api. This is causing a significant performance regression because of the extra overhead. The longer term goal is to move as much of the performance critical passes to operate in rust which will eliminate this overhead. But in the meantime we can mitigate the performance overhead by changing the Python access patterns to avoid the operation object creation. This commit adds some new getter methods to DAGOpNode to give access to the inner rust data so that we can avoid the extra overhead. As a proof of concept this updates the unitary synthesis pass in isolation. Doing this fixes the regression caused by #12459 for that pass. We can continue this migration for everything else in follow up PRs. This commit is mostly to establish the pattern and add the python space access methods. * Remove unused import * Add path to avoid StandardGate conversion in circuit_to_dag * Add fast path through dag_to_circuit
This commit updates the commutative cancellation and commutation analysis transpiler pass. It builds off of Qiskit#12692 to adjust access patterns in the python transpiler path to avoid eagerly creating a Python space operation object. The goal of this PR is to mitigate the performance regression on these passes introduced by the extra conversion cost of Qiskit#12459.
This commit moves to use rust gates for the ConsolidateBlocks transpiler pass. Instead of generating the unitary matrices for the gates in a 2q block Python side and passing that list to a rust function this commit switches to passing a list of DAGOpNodes to the rust and then generating the matrices inside the rust function directly. This is similar to what was done in Qiskit#12650 for Optimize1qGatesDecomposition. Besides being faster to get the matrix for standard gates, it also reduces the eager construction of Python gate objects which was a significant source of overhead after Qiskit#12459. To that end this builds on the thread of work in the two PRs Qiskit#12692 and Qiskit#12701 which changed the access patterns for other passes to minimize eager gate object construction.
* Avoid Python op creation in commutative cancellation This commit updates the commutative cancellation and commutation analysis transpiler pass. It builds off of #12692 to adjust access patterns in the python transpiler path to avoid eagerly creating a Python space operation object. The goal of this PR is to mitigate the performance regression on these passes introduced by the extra conversion cost of #12459. * Remove stray print * Don't add __array__ to DAGOpNode or CircuitInstruction
This commit moves to use rust gates for the ConsolidateBlocks transpiler pass. Instead of generating the unitary matrices for the gates in a 2q block Python side and passing that list to a rust function this commit switches to passing a list of DAGOpNodes to the rust and then generating the matrices inside the rust function directly. This is similar to what was done in Qiskit#12650 for Optimize1qGatesDecomposition. Besides being faster to get the matrix for standard gates, it also reduces the eager construction of Python gate objects which was a significant source of overhead after Qiskit#12459. To that end this builds on the thread of work in the two PRs Qiskit#12692 and Qiskit#12701 which changed the access patterns for other passes to minimize eager gate object construction.
This commit updates the BasisTranslator transpiler pass. It builds off of Qiskit#12692 and Qiskit#12701 to adjust access patterns in the python transpiler path to avoid eagerly creating a Python space operation object. The goal of this PR is to mitigate the performance regression introduced by the extra conversion cost of Qiskit#12459 on the BasisTranslator.
Summary
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 aCircuitInstruction
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
, andPyOperation
) a new Rust traitOperation
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 theOperationType
enum which wraps all 4 (and is used as the type forCircuitInstruction.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 underlyingQuantumCircuit
's responsibility has to move to Rust as well. Primarily this involves theParameterTable
which was an internal class for tracking which instructions in the circuit have aParameterExpression
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).Details and comments
Fixes: #12205
TODO: