Skip to content

Conversation

@augustehirth
Copy link
Contributor

@augustehirth augustehirth commented May 20, 2022

This experiment has some code in cirq-core/cirq/experiments, along with a docs notebook. Both have been moved here, into recirq/rabi_oscillations and docs/rabi_oscillations respectively

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@augustehirth
Copy link
Contributor Author

@mpharrigan @dstrain115 Requesting review by comment because I don't think I can through the github interface 👍

@mpharrigan
Copy link
Collaborator

I'm hesitant to add a new top-level module for each little thing like this. Can we combine it with somewhere else?

@mpharrigan
Copy link
Collaborator

benchmarks/ beyond_classical/ readout_scan/ and this (rabi_oscillation/) should probably share a recirq top-level module. Since they're all little demos/diagnostics and not supporting a published research thing.

@mpharrigan
Copy link
Collaborator

Can we put this code in benchmarks/? @dstrain115

@dstrain115
Copy link
Collaborator

Maybe we could add "tools/" ?

@dstrain115
Copy link
Collaborator

Or adding it to benchmarks seems acceptable as well.

@augustehirth
Copy link
Contributor Author

augustehirth commented May 20, 2022

Moved to benchmarks/ directory

@@ -0,0 +1,488 @@
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

Line #8.            v = hashlib.sha256(str(q).encode()).digest()[0] / 256

lol why are we doing this hashing? can't we just put a number


Reply via ReviewNB

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure. Maybe the original author intended it to hide the actual value of $v$

Copy link
Collaborator

@dstrain115 dstrain115 left a comment

Choose a reason for hiding this comment

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

This is fine with me, modulo one comment.

"id": "b2TkL28AmBSQ"
},
"source": [
"## 3. The ReCirq experiment\n",
Copy link
Collaborator

Choose a reason for hiding this comment

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

This still doesn't really explain this at all. I would consider linking to the wikipedia article and maybe explaining why we want to do this?

@mpharrigan mpharrigan merged commit dcfe491 into quantumlib:master May 31, 2022
@mpharrigan mpharrigan added the area/docs Concerns documentation label May 31, 2022
CirqBot pushed a commit to quantumlib/Cirq that referenced this pull request Jun 14, 2022
Rabi Oscillations have been moved per quantumlib/ReCirq#292.
Also update the intro notebook to not use now-moved recirq experiment (replaced with single_qubit_state_tomography)
Also ran a formatter and cleared outputs from intro notebook

Only merge with redirect cl/452988155
augustehirth added a commit to augustehirth/Cirq that referenced this pull request Jun 15, 2022
…tumlib#5448)

Rabi Oscillations have been moved per quantumlib/ReCirq#292.
Also update the intro notebook to not use now-moved recirq experiment (replaced with single_qubit_state_tomography)
Also ran a formatter and cleared outputs from intro notebook

Only merge with redirect cl/452988155
@augustehirth augustehirth deleted the rabi_oscillations branch June 23, 2022 00:14
@augustehirth augustehirth restored the rabi_oscillations branch June 23, 2022 00:15
@augustehirth augustehirth deleted the rabi_oscillations branch June 23, 2022 00:16
rht pushed a commit to rht/Cirq that referenced this pull request May 1, 2023
…tumlib#5448)

Rabi Oscillations have been moved per quantumlib/ReCirq#292.
Also update the intro notebook to not use now-moved recirq experiment (replaced with single_qubit_state_tomography)
Also ran a formatter and cleared outputs from intro notebook

Only merge with redirect cl/452988155
@augustehirth augustehirth restored the rabi_oscillations branch August 25, 2023 17:56
harry-phasecraft pushed a commit to PhaseCraft/Cirq that referenced this pull request Oct 31, 2024
…tumlib#5448)

Rabi Oscillations have been moved per quantumlib/ReCirq#292.
Also update the intro notebook to not use now-moved recirq experiment (replaced with single_qubit_state_tomography)
Also ran a formatter and cleared outputs from intro notebook

Only merge with redirect cl/452988155
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/docs Concerns documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants