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

SparsePauliOp casting of coeffs causes errors for dtype=real #8992

Closed
chriseclectic opened this issue Oct 25, 2022 · 3 comments · Fixed by #8998
Closed

SparsePauliOp casting of coeffs causes errors for dtype=real #8992

chriseclectic opened this issue Oct 25, 2022 · 3 comments · Fixed by #8998
Labels
bug Something isn't working

Comments

@chriseclectic
Copy link
Member

Environment

  • Qiskit Terra version: 0.22
  • Python version: All
  • Operating system: All

What is happening?

The current init for SparsePauliOp has handling for storing coefficient arrays using their input dtype, but this is used to cast the coeffs to that type after moving the Pauli phase to the coeffs. This can cause errors if the dtype is not complex.

How can we reproduce the issue?

Here is an example which gives an incorrect answer:

# Initialize ops, with coeffs with dtype double
a = SparsePauliOp(['X'], np.array([1]))
b = SparsePauliOp(['Y'], np.array([1]))
a.dot(b)

This raises a warning

# Raises waring
.../sparse_pauli_op.py:131: ComplexWarning: Casting complex values to real discards the imaginary part
  self._coeffs = np.asarray((-1j) ** (phase - count_y) * coeffs, dtype=coeffs.dtype)

and returns the incorrect value

SparsePauliOp(['Z'],
              coeffs=[0])

What should happen?

For the above example the correct answer should be

SparsePauliOp(['Z'],
              coeffs=[0.+1.j])

Any suggestions?

Removing the explicit dype in the line from the warning should fix this, which would allow double coeffs to be converted to complex if required.

I suspect this support was added in the first place to handle object arrays for parameters, and I think that should still be supported if the dtype arg is not used since the asarray should default to an object array in that case.

@chriseclectic chriseclectic added the bug Something isn't working label Oct 25, 2022
@jakelishman
Copy link
Member

Yeah, seems like the allowed dtypes here should only be np.complex128 and object - any other numeric dtype should perhaps be raised to complex to avoid this error.

@ikkoham: did you have more dtypes in mind when you wrote that part of the handling?

@ikkoham
Copy link
Contributor

ikkoham commented Oct 25, 2022

Ever wanted to use a float to save data? Well, no.

@ikkoham
Copy link
Contributor

ikkoham commented Oct 25, 2022

OK, I'll restrict it to complex and object.

ikkoham added a commit to ikkoham/qiskit-terra that referenced this issue Oct 26, 2022
@mergify mergify bot closed this as completed in #8998 Oct 26, 2022
mergify bot pushed a commit that referenced this issue Oct 26, 2022
mergify bot pushed a commit that referenced this issue Oct 26, 2022
(cherry picked from commit be1e697)
mergify bot added a commit that referenced this issue Oct 26, 2022
(cherry picked from commit be1e697)

Co-authored-by: Ikko Hamamura <ikkoham@users.noreply.github.com>
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

Successfully merging a pull request may close this issue.

3 participants