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 rust representation of the UnitaryGate class #13272

Open
Tracked by #13264
mtreinish opened this issue Oct 3, 2024 · 0 comments · May be fixed by #13759
Open
Tracked by #13264

Add rust representation of the UnitaryGate class #13272

mtreinish opened this issue Oct 3, 2024 · 0 comments · May be fixed by #13759
Assignees
Labels
performance Rust This PR or issue is related to Rust code in the repository
Milestone

Comments

@mtreinish
Copy link
Member

The UnitaryGate class is defined in Python and that means that when we need to work with it from Rust (either for transpiler passes or circuit construction) we treat it as a PyInstruction and that requires that we use Python for interacting with it. For the most part for reading this isn't a huge overhead because a UnitaryGate is a gate defined around a unitary matrix as a numpy array and we typically just need to inspect that matrix which can be done by reference without any copying or conversion. However, on writing (typically creation), which we do a fair amount of in the transpiler and the circuit library, the python overhead is significant. In rust it will be easy to build a UnitaryGate struct that implements the Operation trait and owns an Array2<Complex64>. We might need to think a bit about the python interface and split of responsibilities between the languages because we can generate a numpy array from a owned array but the traits in rust-numpy for doing that either consume the array or copy it.

This weakly depends on #12966 as we'll need to figure out the best way to expand PackedOperation to support this and the pattern we establish in #12966 will inform this.

@mtreinish mtreinish added performance Rust This PR or issue is related to Rust code in the repository labels Oct 3, 2024
@mtreinish mtreinish added this to the 2.0.0 milestone Oct 3, 2024
mtreinish added a commit to mtreinish/qiskit-core that referenced this issue Oct 4, 2024
In the accelerate crate we currently have two places that are building
`UnitaryGate`, the quantum_volume() function that builds a quantum
volume model circuit and the Split2QUnitaries transpiler pass. Currently
this can only be done by calling Python (until Qiskit#13272 is implemented)
and there was a potential optimization we could make to specify the
number of qubits as an argument to the Python constructor. This skipss
the need for a few python operations to compute the number of qubits
from the size of the matrix. These operations are not exceedingly slow,
but as these Python space constructors are often the bottleneck so it
should help runtime performance of these two functions.
github-merge-queue bot pushed a commit that referenced this issue Oct 7, 2024
In the accelerate crate we currently have two places that are building
`UnitaryGate`, the quantum_volume() function that builds a quantum
volume model circuit and the Split2QUnitaries transpiler pass. Currently
this can only be done by calling Python (until #13272 is implemented)
and there was a potential optimization we could make to specify the
number of qubits as an argument to the Python constructor. This skipss
the need for a few python operations to compute the number of qubits
from the size of the matrix. These operations are not exceedingly slow,
but as these Python space constructors are often the bottleneck so it
should help runtime performance of these two functions.
mtreinish added a commit to mtreinish/qiskit-core that referenced this issue Nov 22, 2024
There was a small typing issue in the quantum_volume implementation
where there was a mismatch in the Python type and the rust type. The
UnitaryGate were being added to the circuit as a rust space
PyInstruction instead of a PyGate. This was incorrect as UnitaryGate is
unitary and a gate type in python, this mismatch was causing subsequent
transpilation or anything working with the unitaries in the quantum
volume circuit from rust to mischaracterize the operations as
non-unitary so things like getting the matrix would fail. This commit
corrects the typing so the gates are added as a unitary operation in the
rust typing.

This will get much simpler and less error prone when Qiskit#13272 is
implemented and we have a rust native UnitaryGate type.
github-merge-queue bot pushed a commit that referenced this issue Nov 22, 2024
There was a small typing issue in the quantum_volume implementation
where there was a mismatch in the Python type and the rust type. The
UnitaryGate were being added to the circuit as a rust space
PyInstruction instead of a PyGate. This was incorrect as UnitaryGate is
unitary and a gate type in python, this mismatch was causing subsequent
transpilation or anything working with the unitaries in the quantum
volume circuit from rust to mischaracterize the operations as
non-unitary so things like getting the matrix would fail. This commit
corrects the typing so the gates are added as a unitary operation in the
rust typing.

This will get much simpler and less error prone when #13272 is
implemented and we have a rust native UnitaryGate type.
mergify bot pushed a commit that referenced this issue Nov 22, 2024
There was a small typing issue in the quantum_volume implementation
where there was a mismatch in the Python type and the rust type. The
UnitaryGate were being added to the circuit as a rust space
PyInstruction instead of a PyGate. This was incorrect as UnitaryGate is
unitary and a gate type in python, this mismatch was causing subsequent
transpilation or anything working with the unitaries in the quantum
volume circuit from rust to mischaracterize the operations as
non-unitary so things like getting the matrix would fail. This commit
corrects the typing so the gates are added as a unitary operation in the
rust typing.

This will get much simpler and less error prone when #13272 is
implemented and we have a rust native UnitaryGate type.

(cherry picked from commit 4bb1432)
github-merge-queue bot pushed a commit that referenced this issue Nov 22, 2024
There was a small typing issue in the quantum_volume implementation
where there was a mismatch in the Python type and the rust type. The
UnitaryGate were being added to the circuit as a rust space
PyInstruction instead of a PyGate. This was incorrect as UnitaryGate is
unitary and a gate type in python, this mismatch was causing subsequent
transpilation or anything working with the unitaries in the quantum
volume circuit from rust to mischaracterize the operations as
non-unitary so things like getting the matrix would fail. This commit
corrects the typing so the gates are added as a unitary operation in the
rust typing.

This will get much simpler and less error prone when #13272 is
implemented and we have a rust native UnitaryGate type.

(cherry picked from commit 4bb1432)

Co-authored-by: Matthew Treinish <[email protected]>
@gadial gadial self-assigned this Jan 23, 2025
mtreinish added a commit to mtreinish/qiskit-core that referenced this issue Jan 30, 2025
This commit expands the rust circuit data model to include a definition
of UnitaryGate. This is a more abstract operation than what we've
defined so far in Rust, a gate represented solely by it's unitary matrix
but during normal transpilation we create and interact with these
operations for optimization purposes so having a native representation
in rust is necessary to avoid calling to Python when working with them.

This introduces a new UnitaryGate struct which represents the unpacked
operation. It has 3 internal storage variants based on either an ndarray
arbitrary sized array, a 2x2 nalgebra fixed sized array, or a 4x4
nalgebra fixed sized array. From the python perspective these all look
the same, but being able to internally work with all 3 variants lets us
optimize the code paths for 1q and 2q unitary gates (which are by far
more common) and avoid one layer of pointer indirection.

When stored in a circuit the packed representation is just a pointer to
the actual UnitaryGate which we put in a Box. This is necessary because
the struct size is too large to fit in our compact representation of
operations. Even the ndarray which is a heap allocated type requires more
than our allotted space in a PackedOperation so we need to reduce it's
size by putting it in a Box.

The one major difference here from the previous python based
representation the unitary matrix was stored as an object type parameter
in the PackedInstruction.params field, which now it is stored internally
in the operation itself. There is arguably a behavior change around
this because it's no longer possible to mutate the array of a
UnitaryGate in place once it's inserted into the circuit. While doing
this was horribly unsound, because there was no guardrails for doing it
a release note is added because there is a small use case where it would
have worked and it wasn't explicitly documented.

Closes Qiskit#13272
mtreinish added a commit to mtreinish/qiskit-core that referenced this issue Jan 30, 2025
This commit expands the rust circuit data model to include a definition
of UnitaryGate. This is a more abstract operation than what we've
defined so far in Rust, a gate represented solely by it's unitary matrix
but during normal transpilation we create and interact with these
operations for optimization purposes so having a native representation
in rust is necessary to avoid calling to Python when working with them.

This introduces a new UnitaryGate struct which represents the unpacked
operation. It has 3 internal storage variants based on either an ndarray
arbitrary sized array, a 2x2 nalgebra fixed sized array, or a 4x4
nalgebra fixed sized array. From the python perspective these all look
the same, but being able to internally work with all 3 variants lets us
optimize the code paths for 1q and 2q unitary gates (which are by far
more common) and avoid one layer of pointer indirection.

When stored in a circuit the packed representation is just a pointer to
the actual UnitaryGate which we put in a Box. This is necessary because
the struct size is too large to fit in our compact representation of
operations. Even the ndarray which is a heap allocated type requires more
than our allotted space in a PackedOperation so we need to reduce it's
size by putting it in a Box.

The one major difference here from the previous python based
representation the unitary matrix was stored as an object type parameter
in the PackedInstruction.params field, which now it is stored internally
in the operation itself. There is arguably a behavior change around
this because it's no longer possible to mutate the array of a
UnitaryGate in place once it's inserted into the circuit. While doing
this was horribly unsound, because there was no guardrails for doing it
a release note is added because there is a small use case where it would
have worked and it wasn't explicitly documented.

Closes Qiskit#13272
@mtreinish mtreinish linked a pull request Jan 30, 2025 that will close this issue
@1ucian0 1ucian0 assigned mtreinish and unassigned gadial Jan 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Rust This PR or issue is related to Rust code in the repository
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants