-
Notifications
You must be signed in to change notification settings - Fork 61
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
Expectation values from samples #675
Merged
Merged
Changes from all commits
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
5505baf
Added expectation_from_samples to SymbolicHamiltonian
AlejandroSopena a345747
Added expectation_from_samples to Hamiltonian
AlejandroSopena b3d03d5
Added docstrings to AbstractHamiltonian
AlejandroSopena 1ff1476
Fix expectation_from_samples
AlejandroSopena 6dd7c20
Added test for Hamiltonian.expectation_from_samples
AlejandroSopena fa7dbb9
Added test for SymbolicHamiltonian.expectation_from_samples
AlejandroSopena 16e929d
Fix tests
AlejandroSopena 1944f87
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 2ffb264
Update tests
AlejandroSopena 72c1ad0
Renamed variables
AlejandroSopena ede207f
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 2112762
Merge branch 'master' into Expectation_Value
scarrazza e47d4a1
Fix tests
AlejandroSopena 0409f2f
Changed qubit_map type
AlejandroSopena dbb2bfb
Added CircuitResult.expectation_from_samples(hamiltonian)
AlejandroSopena 2fab707
Added test for CircuitResult.expectation_from_samples
AlejandroSopena fff6364
Fix import numpy
AlejandroSopena 7a3e198
Merge branch 'master' into Expectation_Value
AlejandroSopena 8c4109a
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here you can avoid the conversion from binary to decimal if
freq
is given in decimal. If these frequencies are result of some circuit execution, the user can doresult.frequencies(binary=False)
to get this without much effort.Even if you get the frequencies in binary, I believe you can still avoid the for loops if the operation is vectorized. For example something like
I would also find a better name for the variable
O
because it does not look very nice in the code. I generally avoid single letter names and capitals in local variables.O
is even more confusing because it's very close to0
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the suggestion. The user can include a
qubit_map
that indicates which qubit each position in the counts refers to. For example, let's consider a 6 qubit circuit where we only measure qubits 1, 3 and 5 because we want to get an expected value of an observable that only acts on those qubits. We will then get counts for the states 000, 001, 010 ...111. When converting these numbers to decimal we have to take into account thequbit_map
because the first position indicates qubit 1, the second position indicates qubit 3 and the third position indicates qubit 5. With this in mind, I do not see a clear way to do it in the way you propose. If you have any suggestion, let me know and I will implement it.I agree that the variable names are not appropriate so I have made some changes. Let me know what you think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the explanation, you are right. I think some part of this calculation can still be vectorized, even with the
qubit_map
, but I am not sure if you gain anything.If you expect
freq
to be a result of circuit execution, in principle you could instead take the theCircuitResult
object, which is what circuit execution returns and contains both the frequencies and the qubit_map. This could be simpler in some cases as the user would not need to specify the qubit map twice, it would be read directly from the measurement gate in the circuit. However, what you are doing here is more flexible because it also works when frequencies do not come from a circuit.A way to have both approaches is to define an additional method
CircuitResult.expectation_from_samples(hamiltonian)
which calls the method you defined here. So one can doI am not sure if this is useful, let me know what you think.
Thanks, looks better now.