Skip to content

Attempt to Fix PauliSumCollector.collect_async warning under notebook(short-term solution) #4004

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

Merged
merged 15 commits into from
Jul 19, 2021
Merged

Attempt to Fix PauliSumCollector.collect_async warning under notebook(short-term solution) #4004

Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
6d957d6
add nest_asyncio to solve collector under notebook environment
BichengYing Apr 8, 2021
c8604bf
Merge branch 'master' into async_warning
BichengYing Apr 8, 2021
c6cbbfc
Merge branch 'master' into async_warning
BichengYing Apr 8, 2021
b2d9269
Fix the typo in clifford_gate::test_commutes_pauli
BichengYing May 29, 2021
0234dda
Revert "Fix the typo in clifford_gate::test_commutes_pauli"
BichengYing May 29, 2021
7596136
Merge remote-tracking branch 'upstream/master' into master
BichengYing Jun 5, 2021
3121748
Merge remote-tracking branch 'upstream/master' into master
BichengYing Jun 20, 2021
566db2f
Merge remote-tracking branch 'upstream/master' into master
BichengYing Jun 24, 2021
8b8125e
Merge branch 'master' into async_warning
BichengYing Jul 2, 2021
3c39402
Add warning above the cell
BichengYing Jul 2, 2021
a3de6d3
Fix for nit
BichengYing Jul 6, 2021
17d9408
Merge branch 'master' into async_warning
CirqBot Jul 7, 2021
ed9f30c
Merge branch 'master' into async_warning
BichengYing Jul 7, 2021
f5d15d5
Merge branch 'master' into async_warning
BichengYing Jul 14, 2021
7e50b9a
Merge branch 'master' into async_warning
BichengYing Jul 18, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 24 additions & 1 deletion docs/operators_and_observables.ipynb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -802,6 +802,29 @@
"The pattern for computing $\\text{Tr} [ \\rho O ]$ is shown below."
]
},
{
"cell_type": "code",
"execution_count": null,
"metadata": {
"id": "7d55b4675db8"
},
"outputs": [],
"source": [
"!pip install nest-asyncio\n",
"# for running collector under jupyter environment\n",
"import nest_asyncio\n",
"nest_asyncio.apply() "
]
},
{
"cell_type": "markdown",
"metadata": {
"id": "5d8e48230d73"
},
"source": [
"**Warning: If you run this notebook in a colab environment with \"Run all\", the cell below may hang forever or you may encounter crash. Please manually run this cell either by pressing the play button to the left of the code, or by using the keyboard shortcut \"Command/Ctrl+Enter\".**"
]
},
{
"cell_type": "code",
"execution_count": null,
Expand All @@ -814,7 +837,7 @@
"collector = cirq.PauliSumCollector(circuit, psum, samples_per_term=10_000)\n",
"\n",
"# Provide a sampler. See also: collector.collect(...).\n",
"collector.collect_async(sampler=cirq.Simulator())\n",
"collector.collect(sampler=cirq.Simulator())\n",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I tried to run this cell, but it's taking an unreasonably long time to complete (5+ minutes). To minimize the impact on test times, we should reduce samples_per_term to a number that allows this to complete in 30 seconds or less.

If this requires choosing an unrealistically low value for samples_per_term, please also add a comment explaining why we're using the new value.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That is strange. It is super quick to run that. In google colab that line takes less than 300ms.
image
In the github action, the whole notebook execution is like one and half minutes.
image

I wonder why it takes you so long time to complete. (I guess maybe related to live lock?) Could you show your test environment?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm running this directly from the Colab link (modified to point to the latest commit on this PR) and the execution consistently hangs or crashes - I haven't had a run complete successfully yet.

Logs include IndexError: pop from an empty deque, but it's unclear whether that's coming from the Colab restart or from asyncio.

Copy link
Collaborator

Choose a reason for hiding this comment

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

For clarity, I've been running the linked colab with the Runtime > Run all command.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Interesting. Yes, if I click Run all, it will fail. But if click Run before that Cell and then manually shift+Enter run that cell can be finished quickly. I guess Run all added another layer of running environment caused this. I will take a look to understand this better.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Interesting. Yes, if I click Run all, it will fail. But if click Run before that Cell and then manually shift+Enter run that cell can be finished quickly.

I can reproduce this result - Run before and then running this cell also succeeds for me. Thanks for digging into this!

Copy link
Collaborator Author

@BichengYing BichengYing Apr 19, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I still do not understand why this happens. What I found is the TensorFlow Federated colab has encountered similar asynchrony issue and they used nest_asyncio as well for solution. I tried to use Run all their colab, and they do not have similar issue :/

"\n",
"# Estimate the observable.\n",
"energy = collector.estimated_energy()\n",
Expand Down