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

Clarify docstring: Pauli-types conjugate() is complex conjugate, not Hermitian conjugate #13051

Closed
aeddins-ibm opened this issue Aug 28, 2024 · 4 comments · Fixed by #13312
Closed
Labels
bug Something isn't working documentation Something is not clear or an error documentation good first issue Good for newcomers

Comments

@aeddins-ibm
Copy link
Contributor

Environment

  • Qiskit version: 1.2
  • Python version: 3.11
  • Operating system: macOS

What is happening?

Not strictly a bug, just a doc clarification.

The docstring for the conjugate method of Pauli is:

Return the conjugate of each Pauli in the list.

and for SparsePauliOp is

Return the conjugate of the SparsePauliOp.

Personally, when I hear "conjugate of an operator" I think Hermitian conjugate. To avoid confusion with Hermitian conjugate, can we change the docstring of conjugate for Pauli, SparsePauliOp, etc to specify explicitly:

Return the complex conjugate of each Pauli in the list.

and

Return the complex conjugate of the SparsePauliOp.

(It's also weird there's a reference to a "list" in the docstring for a single Pauli, but I'm not concerned about that here).

How can we reproduce the issue?

n/a

What should happen?

n/a

Any suggestions?

No response

@aeddins-ibm aeddins-ibm added the bug Something isn't working label Aug 28, 2024
@Cryoris
Copy link
Contributor

Cryoris commented Oct 1, 2024

I would agree, that can be confusing. Would you like to open an issue to fix this? 🙂

@Cryoris Cryoris added the documentation Something is not clear or an error documentation label Oct 1, 2024
@javabster javabster added the good first issue Good for newcomers label Oct 10, 2024
@boonware
Copy link
Contributor

@Cryoris Can you assign this to me, please? A fix is in this PR: #13312

@aeddins-ibm
Copy link
Contributor Author

Thanks all -- would it be worth doing this for SparsePauliOp as well?

It would be updating the description of conjugate that shows up here in the docs: https://docs.quantum.ibm.com/api/qiskit/qiskit.quantum_info.SparsePauliOp#conjugate
Image

However I do not see where this string is defined in qiskit's code.

@boonware
Copy link
Contributor

@aeddins-ibm Indeed, it looks like there are more references that could do with updating. I have created an issue and will tackle this soon: #13322

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Something is not clear or an error documentation good first issue Good for newcomers
Projects
Status: Done
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants