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.
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
Create a new method to return the final state vector array instead of wrapping it #623
Create a new method to return the final state vector array instead of wrapping it #623
Changes from 12 commits
6d12636
6705a06
736d45b
8c84f39
6337d59
9b9e426
2c7c06b
585a996
16b5ab6
575ba8d
7cdda0c
5794472
d16dfa0
a1e5a22
0009bc4
1277509
8627951
6c17d8d
7de5803
7bad066
fab46a2
b437035
3b7c878
30441c8
fe0090a
c1dfbd1
72bf545
2600dac
393a7a3
510e4f0
1bd151c
e8df883
cb32edb
6b159a9
2da0548
343895c
a1025aa
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
This violates the API defined by
cirq.SimulatesFinalState.simulate_sweep_iter
. If there are plans to modify that function as well, please link the relevant Cirq PR. (The requiredcirq
version for qsim will also need to be updated if this is the case.)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.
it doesn't violate the API. but it changes the internal data representation just for this if and only if
as_1d_state_vector = True
which by default isFalse
so nothing changes unless the caller explicitly wants the 1D representation. This is done only for the qsim simulator.The 1D representation is used for only one reason and that is to report the result, because the normal representation is a tensor that has number of dimensions equal to the
num_qubits
which breaks when the number of qubits is greater than the limit on numpy array dimensions (see issue in docstring & PR description)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.
QSimSimulator
inherits fromcirq.SimulatesFinalState
, whosesimulate_sweep_iter
method does not have this new argument. Even though the implementation here can accept any valid input to the function of the parent class, it's still a violation of the API.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.
I changed the approach to adding a new method rather than extending the existing API.