Skip to content

Conversation

@95-martin-orion
Copy link
Collaborator

Fixes #503.

*_sample and *_sample_final need to be different functions because they have incompatible behaviors:

  • *_sample (the existing methods) sample from measurements in a single execution of a circuit.
  • *_sample_final (the new methods) sample repeatedly from the final state of a circuit.

Copy link
Collaborator

@sergeisakov sergeisakov left a comment

Choose a reason for hiding this comment

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

LGTM. Does it make sense to add some non-trivial test?

@95-martin-orion
Copy link
Collaborator Author

LGTM. Does it make sense to add some non-trivial test?

I considered adding tests for #502, but a 33-qubit circuit of any depth would be excessive for a unit test. Other than that, the existing Python tests cover repeated sampling pretty thoroughly - is there anything extra you see that needs testing?

@sergeisakov
Copy link
Collaborator

LGTM. Does it make sense to add some non-trivial test?

I considered adding tests for #502, but a 33-qubit circuit of any depth would be excessive for a unit test. Other than that, the existing Python tests cover repeated sampling pretty thoroughly - is there anything extra you see that needs testing?

It seems that the tests are sort of trivial. For instance, the circuit in test_qsim_run_vs_cirq_run (line 405 in qsimcirq_test.py) just swaps the |0> amplitude and the run method always measures the same bit string. I was thinking of, say, a 4- or 5-qubit circuit with less trivial output and running a few thousands repetitions and comparing the measurement histogram with the actual probabilities. I am not sure if it makes sense to add such a test.

@95-martin-orion
Copy link
Collaborator Author

The pybind call is simple enough that I think it's safe to rely on the C++ tests for verifying sampling behavior. The only new complexity on the Python side is in the bit-management of converting raw_results to full_results - I don't think a circuit with multiple possible sampling outcomes adds much coverage in this regard.

For reference, the existing test_qsim_run_vs_cirq_run uses a single-outcome circuit to specifically examine the behavior of that outcome. The result is asymmetric (0101) to check for bit-ordering errors, and multiple measurements are extracted to verify that mapping from raw data to keyed results is working.

@sergeisakov
Copy link
Collaborator

SGTM

@95-martin-orion 95-martin-orion merged commit 15595ec into master Feb 11, 2022
@95-martin-orion 95-martin-orion deleted the qsim-repeat-samples branch February 11, 2022 01:00
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.

Use qsim for repeated sampling

2 participants