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

Return real expectation values in Hamiltonian.expectation_from_samples #1153

Merged
merged 1 commit into from
Jan 10, 2024

Conversation

MatteoRobbiati
Copy link
Contributor

@MatteoRobbiati MatteoRobbiati commented Jan 9, 2024

A very small fix (as the title says).

Checklist:

  • Reviewers confirm new code works as expected.
  • Tests are passing.
  • Coverage does not decrease.
  • Documentation is updated.

Copy link

codecov bot commented Jan 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (0eeb6ae) 100.00% compared to head (351eff8) 100.00%.

Additional details and impacted files
@@            Coverage Diff            @@
##            master     #1153   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           67        67           
  Lines         9588      9588           
=========================================
  Hits          9588      9588           
Flag Coverage Δ
unittests 100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@MatteoRobbiati MatteoRobbiati added this to the Qibo 0.2.4 milestone Jan 9, 2024
@renatomello renatomello changed the title Real expv in expectation_from_samples Return real expectation values in expectation_from_samples Jan 9, 2024
@renatomello renatomello changed the title Return real expectation values in expectation_from_samples Return real expectation values in Hamiltonian.expectation_from_samples Jan 9, 2024
@renatomello
Copy link
Contributor

@MatteoRobbiati let me use this PR to ask you if this feature is related to this old issue #586 and if the issue should be closed

@MatteoRobbiati
Copy link
Contributor Author

@MatteoRobbiati let me use this PR to ask you if this feature is related to this old issue #586 and if the issue should be closed

I think that issue was related to a specific feature request (compute more expv with one set of frequencies?).

@BrunoLiegiBastonLiegi
Copy link
Contributor

Thanks @MatteoRobbiati, my only concern is that discarding the imaginary part there might hide possible bugs. Maybe adding a check that the imaginary part is actually zero (or small enough) would be safer, although it will add some overhead...

@MatteoRobbiati MatteoRobbiati self-assigned this Jan 10, 2024
@MatteoRobbiati MatteoRobbiati added this pull request to the merge queue Jan 10, 2024
Merged via the queue into master with commit ea72695 Jan 10, 2024
21 checks passed
@scarrazza scarrazza deleted the realexp branch January 12, 2024 11:51
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.

3 participants