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

quantum_info.Statevector methods do not return the expected custom type #9513

Open
SimoneGasperini opened this issue Feb 1, 2023 · 4 comments
Labels
bug Something isn't working

Comments

@SimoneGasperini
Copy link
Contributor

Environment

  • Qiskit Terra version: 0.22.3
  • Python version: 3.9.15

What is happening?

I'm working on the definition of a custom class MyStatevector which inherits from qiskit.quantum_info.Statevector and implements some further functionalities. The problem is that some of the Statevector methods (e.g. staticmethod, classmethod, but not only) always return a Statevector instance, no matter what type the original caller was.
In general, I don't think this is the desirable and expected behaviour since, as soon as I call one of these methods on a MyStatevector instance, Qiskit silently "converts" it back to a Statevector, preventing me from further using my custom object.

How can we reproduce the issue?

As an example, let's consider the Statevector.conjugate method, implemented in qiskit.quantum_info as follows:

def conjugate(self):
    """Return the conjugate of the operator."""
    return Statevector(np.conj(self.data), dims=self.dims())

This is the definition of my custom class:

from qiskit.quantum_info import Statevector

class MyStatevector(Statevector):

    def __init__(self, data, dims=None):
        super().__init__(data=data, dims=dims)
    
    def mymethod(self):
        print('mymethod')

The following code raises an error:

psi = MyStatevector([1j, 0])
conj = psi.conjugate()
conj.mymethod()

AttributeError: 'Statevector' object has no attribute 'mymethod'

What should happen?

The code should run with no errors and print the string 'mymethod'.
The type of conj is always expected to be the same of the caller psi.

Any suggestions?

In this simple case, it should be enough to fix the definition of Statevector.conjugate as follows:

def conjugate(self):
    """Return the conjugate of the operator."""
    return self.__class__(np.conj(self.data), dims=self.dims())

For static/class methods, fixing this could require a bit of work, but I think it could be very useful as well.

@SimoneGasperini SimoneGasperini added the bug Something isn't working label Feb 1, 2023
@jakelishman
Copy link
Member

jakelishman commented Feb 1, 2023

In simple cases like this, it is possible to update, but I'm not super keen on opening the can of worms that is implicitly making Statevector subclass-cooperative. Classes need to be specifically designed to be safe for subclasses, and that's not really part of Statevector's API. What sort of use-cases did you have in mind?

Some of the problems that might well arise:

  • which of the attributes/properties of Statevector do you expect to maintain? We may rely on certain behaviours that aren't necessarily explicitly mentioned as being class invariants, because this wasn't designed to be subclassed.
  • how should Operator (and other things) know to use additional functionality from your class, and how would it return the right subclass from operations on your class?
  • how would binary operators on Statevector (like __add__) return the correct type? It might matter whether you do a + b or b + a if you're not very careful.

Those aren't necessarily unsolvable issues, I'm just mentioning them as reasons as to why I'm very nervous about trying to move towards a subclassable Statevector.

(For what it's worth, we actually do subclass Statevector ourselves in Aer, but that's really intended for a limited set of functionality.)

@SimoneGasperini
Copy link
Contributor Author

SimoneGasperini commented Feb 7, 2023

Ok yes, thank you for the answer. Now I understand the sort of issues that can arise from making Statevector subclassable.
What I'm trying to do is to extend the Statevector, DensityMatrix, and Operator classes (defined in the quantum_info module) in order to make them work with "symbolic" sympy matrices (instead of numpy arrays). I started trying to inherit from the corresponding Qiskit class but I still need to figure out whether this is the best approach or not.

@jakelishman
Copy link
Member

Introducing symbolic calculation with matrices is a whole other can of worms as well. Honestly, if you're having a play with that sort of thing, I'd probably try building them up as completely separate objects first, and only then maybe start to think about how interop might work - ergonomic and performant (well, as much as can be expected with symbols) can be tricky in their own right.

It's quite difficult to correctly manage the multiple-dispatch needed to allow things like binary methods (e.g. Statevector.__add__ or Operator.whatever(self, statevector)) to work well when each of the operands could have different types. I don't think there's any scope in Qiskit for merging something like that into Terra core - it'd just be too complex for us, given that the core feature of Terra is QuantumCircuit and the transpiler, and stuff built on top of those. If we were starting from scratch, qiskit.quantum_info might even be a completely separate package.

@SimoneGasperini
Copy link
Contributor Author

SimoneGasperini commented Feb 15, 2023

Thanks again for your help! I'm having a try to make something like SymbStatevector(pqc) or SymbOperator(pqc) possible working with sympy and Qiskit (pqc here is a parameterized QuantumCircuit object) --> #4751. As you suggested, I started by defining my own classes without any interop but sticking to Qiskit interface as much as possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants