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

Circuit Execution Outputs #1039

Merged
merged 57 commits into from
Oct 27, 2023
Merged

Circuit Execution Outputs #1039

merged 57 commits into from
Oct 27, 2023

Conversation

BrunoLiegiBastonLiegi
Copy link
Contributor

@BrunoLiegiBastonLiegi BrunoLiegiBastonLiegi commented Oct 10, 2023

As requested in #1020, three objects are implemented that substitute the old CircuitResult object.
Depending on the different conditions, now the circuit execution returns:

  1. no noise, no measurements --> QuantumState
  2. no noise, measurements --> CircuitResult
  3. noise, no measurements --> raise error if density_matrix=False else QuantumState
  4. noise, measurements --> CircuitResult if density_matrix=True else MeasurementOutcomes

where the new CircuitResult(QuantumState, MeasurementOutcomes) object contains both the QuantumState and the MeasurementOutcomes.
nshots is set by default to 1000 now.
The probabilities and the state can be recovered as QuantumState.probabilities() and QuantumState.state(), the frequencies and the samples with MeasurementOutcomes.frequencies() and MeasurementOutcomes.samples(). CircuitResult gives access to all of them in the same way.

Currently the only case that has been left out is measurements with collapse=True.
One possibility to handle them could be:

  • only collapsing measurements present
    --> QuantumState if density_matrix=True
    --> raise Error if density_matrix=False
  • both collapsing and standard measurements present
    --> CircuitResult if density_matrix=True
    --> MeasurementOutcomes if density_matrix=False

Furthermore, handling the Circuit._final_state attribute now is a bit problematic, as depending on the conditions different different things are obtained after execution.

Checklist:

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

@codecov
Copy link

codecov bot commented Oct 13, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Files Coverage Δ
src/qibo/__init__.py 100.00% <ø> (ø)
src/qibo/backends/abstract.py 100.00% <100.00%> (ø)
src/qibo/backends/numpy.py 100.00% <100.00%> (ø)
src/qibo/backends/tensorflow.py 100.00% <100.00%> (ø)
src/qibo/gates/measurements.py 100.00% <100.00%> (ø)
src/qibo/measurements.py 100.00% <100.00%> (ø)
src/qibo/models/circuit.py 100.00% <100.00%> (ø)
src/qibo/models/error_mitigation.py 100.00% <ø> (ø)
src/qibo/models/qgan.py 100.00% <100.00%> (ø)
src/qibo/models/tsp.py 100.00% <ø> (ø)
... and 3 more

📢 Thoughts on this report? Let us know!.

@BrunoLiegiBastonLiegi BrunoLiegiBastonLiegi marked this pull request as ready for review October 16, 2023 09:33
@BrunoLiegiBastonLiegi
Copy link
Contributor Author

Okay this should be ready for review now. I am also working on adding a table to the docs to summarize the circuit execution outcomes depending on the settings.

Copy link
Member

@stavros11 stavros11 left a comment

Choose a reason for hiding this comment

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

Thank you for working on this @BrunoLiegiBastonLiegi. The new objects look good and the results are much cleaner now. I wrote some comments below, note that some of them are hidden because they are many. I still need to have a look in the documentation updates and a more detailed look in the test updates.

Other than that, it may be worth updating this with main and fixing the conflicts every now and then because conflicts will block the merge even if we approve. In principle you can fix once when we are ready to merge but is usually better to maintain this up-to-date with main.

BrunoLiegiBastonLiegi and others added 7 commits October 17, 2023 10:19
Co-authored-by: Stavros Efthymiou <35475381+stavros11@users.noreply.github.com>
Co-authored-by: Stavros Efthymiou <35475381+stavros11@users.noreply.github.com>
Co-authored-by: Stavros Efthymiou <35475381+stavros11@users.noreply.github.com>
Co-authored-by: Stavros Efthymiou <35475381+stavros11@users.noreply.github.com>
Co-authored-by: Stavros Efthymiou <35475381+stavros11@users.noreply.github.com>
Co-authored-by: Stavros Efthymiou <35475381+stavros11@users.noreply.github.com>
Co-authored-by: Stavros Efthymiou <35475381+stavros11@users.noreply.github.com>
@scarrazza
Copy link
Member

@BrunoLiegiBastonLiegi thanks for this. Could you please also consider the possibility to store and load the numpy object that are required in order to reallocate the results classes? This is necessary for the future remote connection mechanism based on http requests.

@scarrazza
Copy link
Member

Could you please include the qibo version in the results objects and remove the backend from the dump and use numpy as default when loading?

@stavros11 stavros11 mentioned this pull request Oct 26, 2023
4 tasks
Copy link
Member

@stavros11 stavros11 left a comment

Choose a reason for hiding this comment

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

I had a quick look at the docs as I have not done this before and it looks good to me. Feel free to merge this and open an issue about the probabilities or anything else we could potentially address in the future.

I will test the qibolab part later and if there is any update required here we can fix in a new PR.

BrunoLiegiBastonLiegi and others added 3 commits October 27, 2023 13:31
Co-authored-by: Stavros Efthymiou <35475381+stavros11@users.noreply.github.com>
Co-authored-by: Stavros Efthymiou <35475381+stavros11@users.noreply.github.com>
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