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

ComposedOp's to_matrix() returns a wrong answer. #9283

Closed
y-masaomi opened this issue Dec 14, 2022 · 1 comment · Fixed by #9316
Closed

ComposedOp's to_matrix() returns a wrong answer. #9283

y-masaomi opened this issue Dec 14, 2022 · 1 comment · Fixed by #9316
Labels
bug Something isn't working mod: opflow Related to the Opflow module

Comments

@y-masaomi
Copy link

y-masaomi commented Dec 14, 2022

Environment

  • Qiskit Terra version:
    qiskit-terra==0.22.3
  • Python version:
    3.10.4
  • Operating system:
    Ubuntu 20.04.5 LTS

What is happening?

ComposedOp's to_matrix() returns a wrong answer.
In addition, ComposedOp's to_matrix() raises an exception if it contains a state vector.

How can we reproduce the issue?

Run the following code:

import numpy as np
from qiskit.opflow import MatrixOp, X, Zero

x = MatrixOp(X.to_matrix())
print((0.5*(x @ X)).to_matrix())

Then, the following will be printed:

[[0.25+0.j 0.  +0.j]
 [0.  +0.j 0.25+0.j]]

In addition to that, the following code is also buggy:

(x @ Zero).to_matrix()

This code raises the following exception:

ValueError                                Traceback (most recent call last)
Cell In[4], line 1
----> 1 (x @ Zero).to_matrix()

File ~/quantum/venv/lib/python3.10/site-packages/qiskit/opflow/list_ops/list_op.py:364, in ListOp.to_matrix(self, massive)
    357 OperatorBase._check_massive("to_matrix", True, self.num_qubits, massive)
    359 # Combination function must be able to handle classical values.
    360 # Note: this can end up, when we have list operators containing other list operators, as a
    361 #       ragged array and numpy 1.19 raises a deprecation warning unless this is explicitly
    362 #       done as object type now - was implicit before.
    363 mat = self.combo_fn(  # pylint: disable=not-callable
--> 364     np.asarray(
    365         [op.to_matrix(massive=massive) * self.coeff for op in self.oplist], dtype=object
    366     )
    367 )
    368 # Note: As ComposedOp has a combo function of inner product we can end up here not with
    369 # a matrix (array) but a scalar. In which case we make a single element array of it.
    370 if isinstance(mat, Number):

ValueError: could not broadcast input array from shape (2,2) into shape (2,)

What should happen?

The former result should be:

[[0.5+0.j, 0. +0.j],
 [0. +0.j, 0.5+0.j]]

The latter result should be:

[0.+0.j, 1.+0.j]

Any suggestions?

This bug is caused by the lack of implementation of to_matrix() for ComposedOp.
ComposedOp doesn't have its own to_matrix(), so it calls to_matrix() of the superclass ListOp.
However, ListOp assumes it is distributive w.r.t. coeff, so the former problem is caused.

The second problem is also caused by the lack of implementation of to_matrix() for ComposedOp.
ListOp assumes all elements of oplist will be converted into the same size matrix; the latter problem happens.

I used the following patch to address this. But I am not sure my patch is correct:

from numbers import Number

import numpy as np
from qiskit.opflow import ComposedOp


def custom_to_matrix():
    def to_matrix(self, massive: bool = False):
        ans = None
        for i, op in enumerate(self.oplist):
            if ans is None:
                ans = op.to_matrix(massive=massive)
            else:
                ans = np.dot(ans, op.to_matrix(massive=massive))

        ans = ans * self.coeff
        if isinstance(ans, Number):
            ans = [ans]

        return np.asarray(ans, dtype=complex)

    ComposedOp.to_matrix = to_matrix
@y-masaomi y-masaomi added the bug Something isn't working label Dec 14, 2022
@woodsp-ibm woodsp-ibm added the mod: opflow Related to the Opflow module label Dec 14, 2022
@Cryoris Cryoris changed the title TensoredOp's to_matrix() returns a wrong answer. ComposedOp's to_matrix() returns a wrong answer. Dec 21, 2022
@Cryoris
Copy link
Contributor

Cryoris commented Dec 21, 2022

Thanks for reporting this @y-masaomi! The linked PR should fix this 🙂

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

Successfully merging a pull request may close this issue.

3 participants