Split the simulators' creation of ActOnArgs and iteration #3970

daxfohl · 2021-03-25T17:26:48Z

Currently all simulators implement _base_iterator, which does two things: it creates the ActOnArgs object, and the iterates the circuit. This PR splits ActOnArgs creation and iteration into two separate functions in each of the simulators. This allows preservation of single-responsibility principle in the simulator methods. It potentially allows moving _core_iterator into the base class to remove that redundancy in a future PR (all four simulators' implementations of _core_iterator are almost identical now).

It also addresses the problem in #3958 with a workaround: the user can now propagate the ActOnArgs to subsequent circuits. See the quantum teleportation example for an example of how to do this. Note that this process works for all simulators, and a function can be written generically to support any of them, so long as the same simulator runs each circuit.

The quantum teleportation is refactored to be implemented in terms of this. This fixes the "hack" of the teleportation in a different way than the classical control tags. Here, the circuit is split into two, and the Bob side is constructed and run after the Alice side based on Alice's measurements. Unlike the tags formulation, this cannot be serialized into a single circuit and requires the Python code to manage the classical coordination. Again, other than the final state vector printout, this example can work generically with any simulator.

Open question: should these functions be public?

google-cla · 2021-03-25T17:26:55Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

daxfohl · 2021-03-25T17:28:42Z

@tonybruguier can you consent here too? Seems like I must have merged wonkily on this one.

tonybruguier · 2021-03-25T18:03:56Z

@googlebot I consent.

cirq/sim/simulator.py

… it directly if supplied.

# Conflicts: # cirq/sim/density_matrix_simulator.py

…everywhere.

daxfohl · 2021-04-19T14:19:12Z

@95-martin-orion I think this would be easier to pull the trigger on if I made create_act_on_args a private function for now and reverted the teleportation example. That would give us time to ponder whether "ActOnArgs" is the right name for this etc, before going all public with it. That would make this purely a refactoring change. Thoughts?

95-martin-orion · 2021-04-19T16:13:08Z

Apologies for the wait! I'll be prioritizing review of this PR this week.

@95-martin-orion I think this would be easier to pull the trigger on if I made create_act_on_args a private function {...}

Let's hold off on this change for now. I'll review the latest changes and see how much of this PR (if anything )should be saved for subsequent PRs.

95-martin-orion

Having reviewed the new changes, I'm inclined to agree with making _create_act_on_args private to avoid "going public" with this behavior for now. Other than that, I'm largely happy with the structure of this PR.

cirq/sim/clifford/clifford_simulator.py

cirq/google/calibration/engine_simulator.py

examples/quantum_teleportation.py

cirq/sim/simulator.py

* create_act_on_args private * Allow CliffordState to take ch_form as an initial state * type annotations * revert quantum teleportation example

daxfohl · 2021-04-19T20:47:57Z

@95-martin-orion The requested changes have been made.

95-martin-orion · 2021-04-19T21:47:42Z

LGTM. Since the big cirq_google PR (#3957) is expected to land today, let's hold this until after it goes through. The fix-up process should be lightweight since this PR only touches one cirq.google file.

95-martin-orion

Balint sent out an email to the cirq-dev group with details on how to sync past the cirq_google change. It shouldn't be too complicated for this PR - let me know if you hit any snags.

daxfohl · 2021-04-20T04:25:42Z

@95-martin-orion Done

…#3970) * Move MPSSimulator to use act_on * Add axes to mps * Finish act-on for mps * lint * Add a copy method for all act_on_args * Split creation of act_on_args and iteration * remove the empty yields * Fix most unit tests * Change sparse simulator to use act_on_args in _run. * Fix clifford simulator * format * optimize imports * Address PR comments * Unit test * Formatting * Rename, format * Fix docs * Update quantum teleportation * Update quantum teleportation * Update quantum teleportation * Make the QubitOrder arg optional * Fix the engine_simulator.py overrides * Fix the engine_simulator.py overrides * Fix the engine_simulator.py overrides * Fix coverage check * Change quantum teleportation example to be generic across simulators. * Change quantum teleportation example to be generic across simulators. * Change qubit_order to an explicit ordering of qubits. * Clean PR issues * Fix type check * Fix unit test * Spelling * small changes * test change * make _core_iterator.sim_state naming consistent * Allow create_act_on_args to accept an ActOnArgs parameter, and return it directly if supplied. * Add tests for `.simulate(ActOnArgs)`. * Add tests for `.simulate(ActOnArgs)`. * lint * Plop qubits and qubit_map into ActOnArgs instead of passing together everywhere. * Change qubits param to Sequence. Clean up teleportation example. * PR comments: * create_act_on_args private * Allow CliffordState to take ch_form as an initial state * type annotations * revert quantum teleportation example * format/lint * format Co-authored-by: Tony Bruguier <[email protected]>

tonybruguier and others added 21 commits March 23, 2021 09:43

Move MPSSimulator to use act_on

8ad4224

Add axes to mps

ffd7b86

Finish act-on for mps

6a28c01

lint

ca0a765

Add a copy method for all act_on_args

988d7ae

Split creation of act_on_args and iteration

b1774b7

remove the empty yields

bfa7ed0

Fix most unit tests

180bb67

Change sparse simulator to use act_on_args in _run.

25efdad

Fix clifford simulator

49f4ae4

format

d11d34f

optimize imports

0ee6526

Address PR comments

f7f792a

Merge branch 'master' into mps

11ac1d2

Unit test

3f6d0e6

Formatting

6c91dd8

Merge branch 'mps' into spliterate

39117a7

Merge branch 'master' into spliterate

1a5fea4

Rename, format

0c8763e

Fix docs

ccc294c

Update quantum teleportation

0edf566

daxfohl requested review from cduck, vtomole and a team as code owners March 25, 2021 17:26

google-cla bot added the cla: no label Mar 25, 2021

google-cla bot added cla: yes Makes googlebot stop complaining. and removed cla: no labels Mar 25, 2021

daxfohl requested a review from 95-martin-orion April 4, 2021 00:30

daxfohl added 3 commits April 5, 2021 00:24

small changes

2102af3

Merge remote-tracking branch 'origin/spliterate' into spliterate

637d88b

test change

452a950

daxfohl commented Apr 10, 2021

View reviewed changes

cirq/sim/simulator.py Outdated Show resolved Hide resolved

cirq/sim/simulator.py Outdated Show resolved Hide resolved

cirq/sim/simulator.py Outdated Show resolved Hide resolved

daxfohl added 9 commits April 10, 2021 11:54

make _core_iterator.sim_state naming consistent

d1a42ad

Allow create_act_on_args to accept an ActOnArgs parameter, and return…

6465a23

… it directly if supplied.

Merge branch 'master' into spliterate

814e82a

# Conflicts: # cirq/sim/density_matrix_simulator.py

Add tests for .simulate(ActOnArgs).

2fe1e8a

Add tests for .simulate(ActOnArgs).

cfb711e

lint

e2063df

Plop qubits and qubit_map into ActOnArgs instead of passing together …

8913bda

…everywhere.

Change qubits param to Sequence. Clean up teleportation example.

d195016

Merge branch 'master' into spliterate

739d69e

95-martin-orion requested changes Apr 19, 2021

View reviewed changes

cirq/sim/clifford/clifford_simulator.py Outdated Show resolved Hide resolved

cirq/google/calibration/engine_simulator.py Outdated Show resolved Hide resolved

examples/quantum_teleportation.py Outdated Show resolved Hide resolved

cirq/sim/simulator.py Outdated Show resolved Hide resolved

daxfohl added 2 commits April 19, 2021 12:45

PR comments:

5ad813c

* create_act_on_args private * Allow CliffordState to take ch_form as an initial state * type annotations * revert quantum teleportation example

format/lint

f319a22

95-martin-orion requested changes Apr 19, 2021

View reviewed changes

daxfohl added 2 commits April 19, 2021 20:57

Merge branch 'master' into spliterate

e5ff9fd

format

0e7ee06

95-martin-orion approved these changes Apr 20, 2021

View reviewed changes

95-martin-orion merged commit 1514cec into quantumlib:master Apr 20, 2021

daxfohl deleted the spliterate branch April 20, 2021 16:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Split the simulators' creation of ActOnArgs and iteration #3970

Split the simulators' creation of ActOnArgs and iteration #3970

daxfohl commented Mar 25, 2021 •

edited

Loading

google-cla bot commented Mar 25, 2021

daxfohl commented Mar 25, 2021

tonybruguier commented Mar 25, 2021

daxfohl commented Apr 19, 2021

95-martin-orion commented Apr 19, 2021

95-martin-orion left a comment

daxfohl commented Apr 19, 2021

95-martin-orion commented Apr 19, 2021

95-martin-orion left a comment

daxfohl commented Apr 20, 2021

Split the simulators' creation of ActOnArgs and iteration #3970

Split the simulators' creation of ActOnArgs and iteration #3970

Conversation

daxfohl commented Mar 25, 2021 • edited Loading

google-cla bot commented Mar 25, 2021

daxfohl commented Mar 25, 2021

tonybruguier commented Mar 25, 2021

daxfohl commented Apr 19, 2021

95-martin-orion commented Apr 19, 2021

95-martin-orion left a comment

Choose a reason for hiding this comment

daxfohl commented Apr 19, 2021

95-martin-orion commented Apr 19, 2021

95-martin-orion left a comment

Choose a reason for hiding this comment

daxfohl commented Apr 20, 2021

daxfohl commented Mar 25, 2021 •

edited

Loading