Skip to content

Adding ReCirq for Lattice Gauge Theory paper accepted to Nature #383

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.

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

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

snichet
Copy link

@snichet snichet commented Apr 22, 2025

In this Recirq Tutorial, I have outlined cirq simulations of the experiments that I conducted as a hardware resident at Google quantum AI. These results are outlined in the arXiv posting https://arxiv.org/abs/2409.17142.

Copy link
Contributor

@eliottrosenberg eliottrosenberg left a comment

Choose a reason for hiding this comment

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

Wow, thanks Tyler!! This is very thorough! I also sent you some very minor comments on the .ipynb notebook.

@eliottrosenberg
Copy link
Contributor

@dstrain115 I don't have write access to this repo, so maybe you could take a look when you get a chance?

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.

What do you think of "lattice_gauge" instead of "lgt_strings"? It might be easier to figure out what a directory name like that means rather than a guess at the abbreviation.

@@ -0,0 +1,1267 @@
from collections.abc import Iterator, Sequence
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add copyright and apache declaration. See elsewhere in Recirq for an example.

Copy link
Author

Choose a reason for hiding this comment

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

done in new commit

@@ -0,0 +1,171 @@
import cirq
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add copyright and apache declaration.

Copy link
Author

Choose a reason for hiding this comment

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

done in new commit


Args:
a_set: set of input qubits, usually ancillary qubits of a set of plaquettes.
qubit1_relationship: either "a", "u", "r", "d", or "l". indicated that the first qubit in each pair will either
Copy link
Collaborator

Choose a reason for hiding this comment

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

Optional: It might be better to have this as an enum rather than a string, but it's not strictly necessary if that's a large change.

Copy link
Author

Choose a reason for hiding this comment

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

done. in new commit I introduce class QubitNeighbor(Enum)

the qubits in a_set.
qubit2_relationship: ... same but the second qubit in each pair.
"""
if qubit1_relationship == "a":
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if it would be better to have a dictionary with some representation that represents the displacement rather than painfully have hundreds of lines of if/then. For instance displacement["a"]["u"] = (0, -1/2) represents no displacement in column but -1/2 displacement in row_vector.

Copy link
Author

Choose a reason for hiding this comment

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

good suggestion. fixed in new commit

def trotter_odd_zcol_entangle_minimal_qubits(
grid: LGTGrid, extra_z_plaquette_indices: list[tuple[int, int]] = []
):
"""Minimal qubits need not obey device connectivity, used for simulation.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This description could be improved. I am not sure what this function does from this sentence.

Copy link
Author

Choose a reason for hiding this comment

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

fixed in new commit

def trotter_even_xcol_entangle_minimal_qubits(
grid: LGTGrid, extra_plaquette_indices: list[tuple[int, int]] = []
):
"""Minimal qubits need not obey device connectivity, used for simulation.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here. This is the exact same description as the previous function. What's the difference?

Copy link
Author

Choose a reason for hiding this comment

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

fixed in new commit

def trotter_odd_xcol_entangle_minimal_qubits(
grid: LGTGrid, extra_plaquette_indices: list[tuple[int, int]] = []
):
"""Minimal qubits need not obey device connectivity, used for simulation.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same description.

Copy link
Author

Choose a reason for hiding this comment

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

fixed in new commit


def excitation_sep_plaquette_input(
plaq_data: np.ndarray, plaq_rows: int = 3, plaq_cols: int = 2
) -> np.ndarray:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Missing docstring.

Copy link
Author

Choose a reason for hiding this comment

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

fixed in new commit

import numpy as np
import sympy

class LGTGrid:
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a pretty long file. Should we move LGTGrid into its own file?

Copy link
Author

Choose a reason for hiding this comment

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

done in new commit

@mhucka
Copy link
Contributor

mhucka commented Apr 23, 2025

@dstrain115 Should we assign this PR to you?

@snichet
Copy link
Author

snichet commented Apr 24, 2025

What do you think of "lattice_gauge" instead of "lgt_strings"? It might be easier to figure out what a directory name like that means rather than a guess at the abbreviation.

Done

@snichet snichet requested a review from dstrain115 April 24, 2025 03:34
@dstrain115
Copy link
Collaborator

Thanks for addressing the code comments. I think we are almost there. There are some test failures.

Also, the formatting of the notebook is not correct. I would add the standard header to the ipynb. Also, please remove the sys.path and user-specific things at the beginning. I would also move the big code block before the motivation section. Maybe this could be in a section called "Setup".

@snichet
Copy link
Author

snichet commented Apr 29, 2025

Thanks for the comments, @dstrain115!

I made the notebook reformatting by moving the setup cells to separate .py files.

I have been trying to use dev_tools/nbfmt to reformat to get the tests to pass. However I keep getting an error when i try to call this function: Installation of tensorflow-docs is broken, please correct. I've tried different python versions and reinstalled it several times. When I run the function a file is populated in dev_tools/tensorflow-docs.txt with the text tensorflow-docs==2025.2.19.33219. Any advice on how to proceed?

@dstrain115
Copy link
Collaborator

@mhucka Do you have any tips on how to get nbfmt working for Tyler?

@mhucka
Copy link
Contributor

mhucka commented Apr 30, 2025

@mhucka Do you have any tips on how to get nbfmt working for Tyler?

Thanks for flagging this. I'll take a look.

@dstrain115
Copy link
Collaborator

You still have this in your notebook:

"sys.path.append('/Users/tylerac/Documents/VS_Code_Projects/ReCirq Stuff/ReCirq')"

We should not have references to personal home directories in the public notebook.

@snichet
Copy link
Author

snichet commented Apr 30, 2025

You still have this in your notebook:

"sys.path.append('/Users/tylerac/Documents/VS_Code_Projects/ReCirq Stuff/ReCirq')"

We should not have references to personal home directories in the public notebook.

Sorry about that. I took it out, and then put it back in for testing reasons after I made updates and just forgot to remove it again. I'll definitely take it out in the next iteration.

@mhucka
Copy link
Contributor

mhucka commented May 1, 2025

@mhucka Do you have any tips on how to get nbfmt working for Tyler?

PR #387 replaces the nbfmt notebook testing script with nbformat taken from Cirq, which is what nbfmt actually used under the hood. Combined with updates to the dependencies, this should work now for testing notebooks.

@snichet After that PR is merged, could you give dev_tools/nbformat a try?

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.

4 participants