[obs] 4.2 - Basic sampling loop #3855

mpharrigan · 2021-02-25T00:07:12Z

Loop through grouped settings, execute, accumulate bitstrings.

The following PRs will add

More advanced control of how many samples to take per chunk and when to stop
Checkpointing
Higher-level APIs that are more convenient to use

Part of #3647
Follows #3792

balopat

Looks good, picked a bunch of nits.

cirq/work/observable_measurement.py

cirq/work/observable_measurement_test.py

cirq/work/observable_measurement.py

balopat · 2021-02-27T01:12:10Z

cirq/work/observable_measurement.py

+    qubit_to_index = {q: i for i, q in enumerate(qubits)}
+
+    needs_init_layer = False
+    for max_setting in grouped_settings.keys():


we could have the whole (for settings as well) "needs init layer" logic extracted into a private method, not just the state part of it

what do you mean? in any event, I inlined the now-one-line definition of _[determine_]needs_init_layer.

Whether an init layer is required only depends on the init state.

I meant having _determine_needs_init_layer separately seemed like it could have encompassed these (now) 5 lines of code. This method (measure_grouped_settings) is very long, I was trying to chip away extractable pieces so it reads better.

needs_init_layer = _determine_needs_init_layer(grouped_settings.keys())

factored out into a helper function, thanks

balopat · 2021-02-27T01:13:46Z

cirq/work/observable_measurement.py

+    if circuit_sweep is None:
+        circuit_sweep = study.UnitSweep
+    else:
+        circuit_sweep = study.to_sweep(circuit_sweep)


circuit_sweep = study.UnitSweep if circuit_sweep is None else study.to_sweep(circuit_sweep)

balopat · 2021-02-27T01:15:32Z

cirq/work/observable_measurement.py

+        ]
+        resolved_params = _to_sweep(resolved_params)
+        total_samples = repetitions * len(resolved_params)
+        print("Total samples", total_samples)


don't we want to use some proper logging instead of print statements? In that case we can also write tests for the things we print with cirq.testing.assert_logs, but also timestamps, formatting, file specs, etc. can be added on top of this kind of output in general.

Do we have an example of user-facing logging in cirq? The python logging module requires the user to configure something to actually get the log messages to appear and the google cloud stuff produces a lot of logging output.

I'm removing this print statement for now. We can add better logging later.

balopat · 2021-02-27T01:18:40Z

cirq/work/observable_measurement.py

+            program=measurement_param_circuit, params=resolved_params, repetitions=repetitions
+        )
+
+        assert len(results) == len(flippy_meas_specs)


Add a readable error message here. In case we run into this, it will be a bug - some hints would be useful to see what expectation failed, why do we think this equality should hold.

this is just testing that the number of results matches the number of params requested and done defensively because of the zip behavior in the next line. If this equality doesn't hold we have bigger problems. Added an assertion message.

mpharrigan

sorry for the delay, PTAL!

cirq/work/observable_measurement.py

mpharrigan · 2021-03-20T01:20:25Z

cirq/work/observable_measurement.py

+    qubit_to_index = {q: i for i, q in enumerate(qubits)}
+
+    needs_init_layer = False
+    for max_setting in grouped_settings.keys():


what do you mean? in any event, I inlined the now-one-line definition of _[determine_]needs_init_layer.

Whether an init layer is required only depends on the init state.

mpharrigan · 2021-03-20T01:21:05Z

cirq/work/observable_measurement.py

+    if circuit_sweep is None:
+        circuit_sweep = study.UnitSweep
+    else:
+        circuit_sweep = study.to_sweep(circuit_sweep)


mpharrigan · 2021-03-20T01:22:22Z

cirq/work/observable_measurement.py

+        ]
+        resolved_params = _to_sweep(resolved_params)
+        total_samples = repetitions * len(resolved_params)
+        print("Total samples", total_samples)


Do we have an example of user-facing logging in cirq? The python logging module requires the user to configure something to actually get the log messages to appear and the google cloud stuff produces a lot of logging output.

I'm removing this print statement for now. We can add better logging later.

mpharrigan · 2021-03-20T01:24:51Z

cirq/work/observable_measurement.py

+            program=measurement_param_circuit, params=resolved_params, repetitions=repetitions
+        )
+
+        assert len(results) == len(flippy_meas_specs)


this is just testing that the number of results matches the number of params requested and done defensively because of the zip behavior in the next line. If this equality doesn't hold we have bigger problems. Added an assertion message.

cirq/work/observable_measurement_test.py

mpharrigan · 2021-03-20T02:05:46Z

CI failure looks scary but unrelated

balopat · 2021-03-22T17:14:37Z

CI failure looks scary but unrelated

Wow that does look scary. Restarted the jobs to see whether it's a flake. Opened #3951.

mpharrigan

anything else @balopat ?

mpharrigan · 2021-03-24T21:04:52Z

cirq/work/observable_measurement.py

+    qubit_to_index = {q: i for i, q in enumerate(qubits)}
+
+    needs_init_layer = False
+    for max_setting in grouped_settings.keys():


factored out into a helper function, thanks

mpharrigan · 2021-03-24T21:06:56Z

@95-martin-orion since you've been working on similar things

balopat

LGTM

@balopat

Extend the observable measurement "main sampling loop" to use `StoppingCriteria` instances to provide different ways of stopping. Two are built in: Variance and Number of repetitions. Users can write their own by implementing the abstract interface. For `measure_grouped_settings`, the caller must provide a `StoppingCriteria`. Forthcoming, higher-level wrappers will provide a more user-friendly way of taking the more traditional path of `RepetitionsStoppingCriteria`. cc @balopat @95-martin-orion Part of #3647 Follows #3855

[obs] 4.2 - Basic sampling loop

8c55bae

mpharrigan requested review from cduck, vtomole and a team as code owners February 25, 2021 00:07

google-cla bot added the cla: yes Makes googlebot stop complaining. label Feb 25, 2021

mpharrigan requested review from balopat and viathor February 25, 2021 00:07

balopat suggested changes Feb 27, 2021

View reviewed changes

mpharrigan added the area/expectation-value label Mar 17, 2021

mpharrigan added 2 commits March 19, 2021 18:09

Merge remote-tracking branch 'origin/master' into obs-4.2-basics

27e20d1

Review comments

14bfa41

mpharrigan commented Mar 20, 2021

View reviewed changes

re-refactor

6f9a970

mpharrigan commented Mar 24, 2021

View reviewed changes

balopat approved these changes Mar 24, 2021

View reviewed changes

mpharrigan added the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Mar 25, 2021

CirqBot added the front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. label Mar 25, 2021

Merge branch 'master' into obs-4.2-basics

6cff7cd

CirqBot merged commit 24e98fa into quantumlib:master Mar 25, 2021

CirqBot removed automerge Tells CirqBot to sync and merge this PR. (If it's running.) front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. labels Mar 25, 2021

mpharrigan mentioned this pull request Mar 25, 2021

[obs] 4.3 - Stopping criteria #3969

Merged

mpharrigan mentioned this pull request Jun 30, 2021

Observable Measurement - 4 - Sampling Loop #3647

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[obs] 4.2 - Basic sampling loop #3855

[obs] 4.2 - Basic sampling loop #3855

mpharrigan commented Feb 25, 2021 •

edited

balopat left a comment

balopat Feb 27, 2021

mpharrigan Mar 20, 2021

balopat Mar 22, 2021 •

edited

mpharrigan Mar 24, 2021

balopat Feb 27, 2021

mpharrigan Mar 20, 2021

balopat Feb 27, 2021

mpharrigan Mar 20, 2021

balopat Feb 27, 2021

mpharrigan Mar 20, 2021

mpharrigan left a comment

mpharrigan Mar 20, 2021

mpharrigan Mar 20, 2021

mpharrigan Mar 20, 2021

mpharrigan Mar 20, 2021

mpharrigan commented Mar 20, 2021

balopat commented Mar 22, 2021

mpharrigan left a comment

mpharrigan Mar 24, 2021

mpharrigan commented Mar 24, 2021

balopat left a comment

[obs] 4.2 - Basic sampling loop #3855

[obs] 4.2 - Basic sampling loop #3855

Conversation

mpharrigan commented Feb 25, 2021 • edited

balopat left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

balopat Mar 22, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mpharrigan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mpharrigan commented Mar 20, 2021

balopat commented Mar 22, 2021

mpharrigan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mpharrigan commented Mar 24, 2021

balopat left a comment

Choose a reason for hiding this comment

mpharrigan commented Feb 25, 2021 •

edited

balopat Mar 22, 2021 •

edited