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.

Sign up for GitHub

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

Jump to bottom

Snapshot statevector and stabilizer extensions #355

Merged
merged 12 commits into from
Oct 2, 2019
Merged

Snapshot statevector and stabilizer extensions #355

merged 12 commits into from
Oct 2, 2019

Conversation

chriseclectic
Copy link
Member

@chriseclectic chriseclectic commented Sep 23, 2019 •
edited
Loading

Summary

**EDIT: ** breaking PR up into multiple smaller ones. This now just covers statevector and stabilizer snapshots.

Fixes signatures of state snapshots:

Close #301, #302, #369

Details and comments

  • Changes statevector, stabilizer, density matrix snapshot circuit function to not need qubit number since they are always all qubit
  • Changes stabilizer snapshot to only return the list of stabilizer generators rather than the full clifford table.
  • Changes order of stabilizer string serialization so that it works correctly with terra Pauli class qubit order.
  • Extension tests
  • Backend implementation tests
  • Add a line to the Changelog

atilag
atilag reviewed Sep 25, 2019
View reviewed changes
Copy link
Member

@atilag atilag left a comment

Choose a reason for hiding this comment

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

Looks good so far.
Waiting for the tests :)

qiskit/providers/aer/extensions/snapshot_density_matrix.py Outdated Show resolved Hide resolved
src/simulators/qasm/qasm_controller.hpp Outdated Show resolved Hide resolved
src/simulators/statevector/statevector_state.hpp Outdated Show resolved Hide resolved
@chriseclectic chriseclectic force-pushed the snapshot-extensions branch 3 times, most recently from b79af38 to d4b16c4 Compare October 1, 2019 20:15
@chriseclectic chriseclectic changed the title [WIP] Snapshot extensions Snapshot statevector and stabilizer extensions Oct 1, 2019
@chriseclectic
Copy link
Member Author

@atilag will break up into a couple of PRs. This one is ready for review and merging

chriseclectic added a commit to chriseclectic/qiskit-aer that referenced this pull request Oct 1, 2019
@chriseclectic
PR Qiskit#355
0f5dcbe
@chriseclectic chriseclectic mentioned this pull request Oct 1, 2019
Merged
@chriseclectic chriseclectic added this to the 0.3.1 release milestone Oct 1, 2019
atilag
atilag previously approved these changes Oct 2, 2019
View reviewed changes
Copy link
Member

@atilag atilag left a comment

Choose a reason for hiding this comment

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

Nice addition, with a lot of tests! Thanks.
I asked some questions just to have a better understanding of some of the concepts, and to double check that everything is ok.

test/terra/backends/qasm_simulator/qasm_snapshot.py Show resolved Hide resolved
test/terra/backends/qasm_simulator/qasm_snapshot.py
self.compare_counts(result,
circuits,
counts_targets,
delta=0.2 * shots)
Copy link
Member

Choose a reason for hiding this comment

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

Does delta need to grow with the number of shots?

Copy link
Member Author

Choose a reason for hiding this comment

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

Delta needs to grow as number of shots gets smaller

test/terra/backends/qasm_simulator/qasm_snapshot.py Show resolved Hide resolved
test/terra/backends/qasm_simulator/qasm_snapshot.py Outdated Show resolved Hide resolved
test/terra/backends/test_qasm_matrix_product_state.py Show resolved Hide resolved
test/terra/extensions/__init__.py Show resolved Hide resolved
@atilag
Copy link
Member

atilag commented Oct 2, 2019

One of the test failed because of a timeout in MacOS provisioning... I'm rerunning the Job again

@chriseclectic chriseclectic merged commit 54c4319 into Qiskit:master Oct 2, 2019
This was referenced Oct 4, 2019
Closed
Closed
@chriseclectic chriseclectic deleted the snapshot-extensions branch October 4, 2019 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@atilag atilag atilag left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.3.1 release
Development

Successfully merging this pull request may close these issues.

Statevector snapshot extension
2 participants
@chriseclectic @atilag