Qiskit / qiskit

Qiskit is an open-source SDK for working with quantum computers at the level of extended quantum circuits, operators, and primitives.
https://www.ibm.com/quantum/qiskit
Apache License 2.0
5.18k stars 2.35k forks source link

ListOp.to_spmatrix does not work #7021

Open ikkoham opened 3 years ago

ikkoham commented 3 years ago

Information

What is the current behavior?

ListOp.to_spmatrix does not work

Steps to reproduce the problem

from qiskit.opflow import ListOp, X, Z
ListOp([X, Z]).to_spmatrix()

What is the expected behavior?

Suggested solutions

This problem is actually not straight forward, because on the type hint said the return is spmatrix. There is an idea to make the type hint Union[spmatrix, list[spmatrix]], but this is difficult because we need to consider the consistency with other methods. Therefore, I don't have an elegant idea to solve this at the moment, although it would be nice if spmatrix had a higher dimensional tensor.

1ucian0 commented 3 years ago

For completeness, here is the output when reproducing the issue:

from qiskit.opflow import ListOp, X, Z
ListOp([X, Z]).to_spmatrix()
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-4-f61b479a5781> in <module>
      1 from qiskit.opflow import ListOp, X, Z
----> 2 ListOp([X, Z]).to_spmatrix()

~/.../qiskit/opflow/list_ops/list_op.py in to_spmatrix(self)
    381         # Combination function must be able to handle classical values
    382         # pylint: disable=not-callable
--> 383         return self.combo_fn([op.to_spmatrix() for op in self.oplist]) * self.coeff
    384 
    385     def eval(

TypeError: can't multiply sequence by non-int of type 'float'
1ucian0 commented 3 years ago

The type hint seems to come from Aqua time. Maybe @akarazeev or @woodsp-ibm have some insights?

woodsp-ibm commented 3 years ago

The suggested solution states this:

This problem is actually not straight forward, because on the type hint said the return is spmatrix. There is an idea to make the type hint Union[spmatrix, list[spmatrix]], but this is difficult because we need to consider the consistency with other methods.

BUT the typehint is that already

https://github.com/Qiskit/qiskit-terra/blob/deebd5cce63601f5a6bb4aa762baedd9cb4876b3/qiskit/opflow/list_ops/list_op.py#L374-L383

The issue is more that when its a list of sparse objects the multiply is not working as I think was intended assuming this was a numpy array and it would distribute the multiply over all the elements.

The best I could come up with is this for the to_spmatrix()

        ret = self.combo_fn([op.to_spmatrix() for op in self.oplist])
        if isinstance(ret, list):
            ret = np.asarray(ret, dtype=object) * self.coeff
        else:
            ret *= self.coeff
        return ret

This then returns either an spmatrix or a numpy array of spmatrix (instead of a list). Well actually since ListOp can be nested the list/array tself contains either spmatrices or Lists where the Lists are spmatrices and Lists etc i.e. its a recursive structure. As a simple example

lxz = ListOp([X, Z, ListOp([X, Z], coeff=1.5)], coeff=3.6)
lxzm = lxz.to_spmatrix()

the above works if spmatrix code is change to what I put above. If one prints the result it looks like this where the contents of the arrays I enumerated to print them to make sure.

[<2x2 sparse matrix of type '<class 'numpy.complex128'>'
        with 2 stored elements in Compressed Sparse Row format>
 <2x2 sparse matrix of type '<class 'numpy.complex128'>'
        with 2 stored elements in Compressed Sparse Row format>
 array([<2x2 sparse matrix of type '<class 'numpy.complex128'>'
        with 2 stored elements in Compressed Sparse Row format>,
       <2x2 sparse matrix of type '<class 'numpy.complex128'>'
        with 2 stored elements in Compressed Sparse Row format>], dtype=object)]
0::
  (0, 1)        (3.6+0j)
  (1, 0)        (3.6+0j)
1::
  (0, 0)        (3.6+0j)
  (1, 1)        (-3.6+0j)
2::
  (0, 1)        (5.4+0j)
  (1, 0)        (5.4+0j)
  (0, 0)        (5.4+0j)
  (1, 1)        (-5.4+0j)

Bottom line here is that ListOp is complicated as it allows other ListOps to be nested. Subclasses like SummedOp should collapse to a single matrix due to the combo function. But ListOp is just a list and if they are nested the structure can be quite complicated/ragged. As I said the above is what I can come up with at present. I have no idea of the utility in general when it returns this complicated list structure rather than a single matrix.