Skip to content

Conversation

@PietropaoloFrisoni
Copy link
Contributor

@PietropaoloFrisoni PietropaoloFrisoni commented Jul 22, 2025

Context: The docstring of the is_hermitian function was not entirely clear. Specifically, there are some edge cases where this property returns the wrong result. Now this limitation is documented.

Description of the Change: As above.

Benefits: More clarity.

Possible Drawbacks: None that I can think of.

Related GitHub Issues: None.

[sc-95920]

@PietropaoloFrisoni PietropaoloFrisoni marked this pull request as ready for review July 22, 2025 20:29
@codecov
Copy link

codecov bot commented Jul 22, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.68%. Comparing base (bd07c01) to head (11ecd35).
⚠️ Report is 419 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7946      +/-   ##
==========================================
- Coverage   99.68%   99.68%   -0.01%     
==========================================
  Files         542      542              
  Lines       55622    55622              
==========================================
- Hits        55449    55448       -1     
- Misses        173      174       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@PietropaoloFrisoni PietropaoloFrisoni removed the request for review from albi3ro July 23, 2025 12:12
Co-authored-by: Christina Lee <[email protected]>
Copy link
Contributor

@isaacdevlugt isaacdevlugt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know there was a discussion about this on slack, but is this behaviour not fixable? It's odd that we have a user-facing operator property that we know gives incorrect results.

Co-authored-by: Isaac De Vlugt <[email protected]>
@PietropaoloFrisoni
Copy link
Contributor Author

PietropaoloFrisoni commented Jul 24, 2025

@isaacdevlugt Yeah I agree that this is odd. At least now this is documented : )

It seems to me that the only 'secure' way would be to compare the matrix of the operator with the adjoint matrix (which is what qml.is_hermitian does). Maybe it makes sense to schedule this property for deprecation?

@albi3ro
Copy link
Contributor

albi3ro commented Jul 24, 2025

I know there was a discussion about this on slack, but is this behaviour not fixable? It's odd that we have a user-facing operator property that we know gives incorrect results.

If it was 100% accurate, it would be too expensive to be useful, since the only real way to be accurate is by using the full matrix. But potentially we could just remove it entirely.

@isaacdevlugt
Copy link
Contributor

isaacdevlugt commented Jul 24, 2025

This PR is fine to merge, but I would strongly consider deprecating and removing op.is_hermitian because of this behaviour 😅

@PietropaoloFrisoni PietropaoloFrisoni added this pull request to the merge queue Jul 25, 2025
Merged via the queue into master with commit 303bd3f Jul 25, 2025
54 checks passed
@PietropaoloFrisoni PietropaoloFrisoni deleted the clarification_is_hermitian_property branch July 25, 2025 12:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants