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

Statevectors and unitaries should not be stored as lists of complex in Result class #2731

Closed
nonhermitian opened this issue Jul 3, 2019 · 1 comment · Fixed by #3181
Closed
Assignees
Labels
type: enhancement It's working, but needs polishing

Comments

@nonhermitian
Copy link
Contributor

What is the expected enhancement?

Currently the Result class stores statevectors and unitary matrices as lists and nested list of complex numbers, respectively. This however makes little sense as simulators use NumPy arrays, or c-aligned vectors that can directly be instantiated from NumPy arrays, internally. Thus the one starts with aligned data, makes a copy to lists in Result and then makes another copy of the data when doing get_statevector() or get_unitary() that returns a NumPy array. Why not just store the data as NumPy arrays in the Results to begin with?

@nonhermitian nonhermitian added the type: enhancement It's working, but needs polishing label Jul 3, 2019
@mtreinish
Copy link
Member

This is not as straightforward as I had originally assumed because of marshmallow. The internal representation of the statevector and unitary in result is done with marshmallow magic and getting it to keep that as a numpy array is tricky. I'll keep fighting the library to figure out if we can coerce it to store things as numpy arrays internally.

This is just another example of where using marshmallow as a base for our serialization objects is not a good choice. It abstracts away the actual object from us and hides it behind too many layers. It makes this all way more complex than it needs to be and for no real benefit.

mtreinish added a commit to mtreinish/qiskit-core that referenced this issue Oct 2, 2019
This commit adds a new validation field type NumpyArray which is used to
return a deserialized numpy array object. If a native numpy array object
is passed in, that's returned verbatim and if it's a list it's first
deserialized as the normal list field type and then cast as an array.
This enables local simulators to return c aligned vectors or numpy
arrays natively. However, for the online simulator case and for
backwards compatibility the serialized list format needs to be
honored. So if it's not a native numpy array object the previous method
of deserializing a list is performed and that output list is cast as a
numpy array. This new field type is then used in place of the previous
List fields for the statevector and unitary result fields in the
ExperimentResultDataSchema class so that local simulators can leverage
this. As an example of how this can be used the BasicAer statevector
and unitary simulators are also updated in this commit to return numpy
arrays of complex numbers directly instead of the serialized list
format it previously used.

Fixes Qiskit#2731
taalexander pushed a commit that referenced this issue Jan 28, 2020
This commit adds a new validation field type NumpyArray which is used to
return a deserialized numpy array object. If a native numpy array object
is passed in, that's returned verbatim and if it's a list it's first
deserialized as the normal list field type and then cast as an array.
This enables local simulators to return c aligned vectors or numpy
arrays natively. However, for the online simulator case and for
backwards compatibility the serialized list format needs to be
honored. So if it's not a native numpy array object the previous method
of deserializing a list is performed and that output list is cast as a
numpy array. This new field type is then used in place of the previous
List fields for the statevector and unitary result fields in the
ExperimentResultDataSchema class so that local simulators can leverage
this. As an example of how this can be used the BasicAer statevector
and unitary simulators are also updated in this commit to return numpy
arrays of complex numbers directly instead of the serialized list
format it previously used.

Fixes #2731

Co-authored-by: Christopher J. Wood <cjwood@us.ibm.com>
faisaldebouni pushed a commit to faisaldebouni/qiskit-terra that referenced this issue Aug 5, 2020
This commit adds a new validation field type NumpyArray which is used to
return a deserialized numpy array object. If a native numpy array object
is passed in, that's returned verbatim and if it's a list it's first
deserialized as the normal list field type and then cast as an array.
This enables local simulators to return c aligned vectors or numpy
arrays natively. However, for the online simulator case and for
backwards compatibility the serialized list format needs to be
honored. So if it's not a native numpy array object the previous method
of deserializing a list is performed and that output list is cast as a
numpy array. This new field type is then used in place of the previous
List fields for the statevector and unitary result fields in the
ExperimentResultDataSchema class so that local simulators can leverage
this. As an example of how this can be used the BasicAer statevector
and unitary simulators are also updated in this commit to return numpy
arrays of complex numbers directly instead of the serialized list
format it previously used.

Fixes Qiskit#2731

Co-authored-by: Christopher J. Wood <cjwood@us.ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement It's working, but needs polishing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants