-
Notifications
You must be signed in to change notification settings - Fork 126
Add DFL experiment code #443
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
base: main
Are you sure you want to change the base?
Conversation
|
@shashwatk1998 can you add docstrings to public methods and then we can merge this? |
|
Trying to push now with added missing public method docstrings, facing permission issue |
|
@shashwatk1998 what permission issues? |
|
@shashwatk1998 just added you as a collaborator to my fork |
|
thanks @shashwatk1998 I see you managed to push the docstring |
recirq/dfl/dfl_1d.py
Outdated
| mu: float, | ||
| two_qubit_gate="cz", | ||
| ) -> cirq.Circuit: | ||
| """Constructs the Trotter circuit for 1D DFL simulation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's a DFL? Avoid abbreviations or at least define them once per file.
recirq/dfl/dfl_1d.py
Outdated
| def _layer_floquet_cphase_first( | ||
| grid: Sequence[cirq.GridQubit], dt: float, h: float, mu: float | ||
| ) -> cirq.Circuit: | ||
| n = len(grid) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it looks like n is only used here once. Probably cleaner to just have for i in range(len(grid)) below. Same elsewhere.
recirq/dfl/dfl_1d.py
Outdated
| Args: | ||
| grid: The 1D sequence of qubits used in the experiment. | ||
| initial_state: The initial state preparation. Valid values are | ||
| "single_sector" or "superposition". |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here that initial_state and basis should not be stringy if they represent a choice between two values.
recirq/dfl/dfl_experiment.py
Outdated
| ``` | ||
| Args: | ||
| repetitions_post_selection: How many repetitions to use for the gauge invariant initial state at each cycle number. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should explain why it can be a list[int] or int. Same elsewhere.
recirq/dfl/dfl_experiment.py
Outdated
| Note: These default values are used for the 1D experiment. For the 2D experiment, we use | ||
| ``` | ||
| repetitions_post_selection = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do these gigantic lists need to be repeated everywhere?
| import cirq | ||
| import numpy as np | ||
|
|
||
| from . import dfl_experiment as dfl |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No relative imports.
|
|
||
| from . import dfl_experiment as dfl | ||
|
|
||
| if not os.path.isdir("test_circuits"): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please put this in a file. If anyone imports this file, it will create this directory in their working directory, which is not good behavior.
dstrain115
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved after other comments addressed.
| CPHASE = 'cphase' | ||
|
|
||
| class InitialState(enum.Enum): | ||
| """Available initial quantum states for the DFL experiment.""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would add a bit more information as to what the options are for these enums.
| class LGTDFL: | ||
| """ | ||
| A class for simulating Disorder-free localization (DFL) on a 2-dimensional Lattice Gauge | ||
| class InitialState(enum.Enum): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we store these enums in a separate file since they can be shared by 1d and 2d files?
| from recirq.dfl import dfl_1d as dfl | ||
| from recirq.dfl import dfl_2d_second_order_trotter as dfl_2d | ||
|
|
||
| #imort Enums explicitly |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit import
Code to run the experiments for Figures 1 and 3 of arXiv:2410.06557.