Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DOC] More examples for BloqadeLattices docstrings #517

Closed
johnzl-777 opened this issue Jan 23, 2023 · 11 comments
Closed

[DOC] More examples for BloqadeLattices docstrings #517

johnzl-777 opened this issue Jan 23, 2023 · 11 comments
Assignees
Labels
documentation Improvements or additions to documentation unitaryhack

Comments

@johnzl-777
Copy link
Collaborator

Suggested by @fanglifl 馃

Which part of the documentation/examples?

https://queracomputing.github.io/Bloqade.jl/dev/lattices/#References

What content needs to be improved/added?

More code examples should be incorporated into the docstrings to make it easier to figure out how to use certain functions and types

@johnzl-777 johnzl-777 added the documentation Improvements or additions to documentation label Jan 23, 2023
@johnzl-777 johnzl-777 self-assigned this Jan 23, 2023
@AbdullahKazi500
Copy link

AbdullahKazi500 commented May 27, 2023

@johnzl-777 can I Get assigned here Thank you

@johnzl-777
Copy link
Collaborator Author

Hey @AbdullahKazi500 sure thing! Apologies for the delay, super glad to see interest on this issue 馃槃

For some additional context this primarily centers around the different lattice types Bloqade supports (e.g. SquareLattice, RectangularLattice). The goal is to provide some more examples of their usage inside their docstrings. Furthermore, they should be formatted properly so that they render correctly in the web version of the documentation.

You can take advantage of the built in CI tools Bloqade has (if you do .ci/run -h at the base of the repo it'll give you all the possible options) to locally build and deploy the docs to ensure they render properly.

Feel free to reach out should anything come up!

@Amana-L
Copy link
Contributor

Amana-L commented Jun 11, 2023

Hi @johnzl-777,

I have made some small changes. Let me know if it is similar to what you're looking for, or not. I'd be happy to amend it.

Thanks for reviewing!

@AbdullahKazi500
Copy link

Hi @johnzl-777 Please assign this issue to @Amana-L I am not working on it

@johnzl-777
Copy link
Collaborator Author

Hi @Amana-L

The effort is greatly appreciated (I also see you fixed a rather glaring typo with our 2D Lieb Lattice being referred to as a 1D chain lattice!) but I was hoping to see more code examples for usage.

For example, the linear_ramp function that's offered as one of Bloqade's waveform generation functions has a docstring like this: https://queracomputing.github.io/Bloqade.jl/dev/waveform/#BloqadeWaveforms.linear_ramp where there's an example of it being used in code.

In a similar fashion it would be nice to see something like:

julia> generate_sites(KagomeLattice(), 4, 4)

In the docstring for each lattice type. The output doesn't have to be the image of the atom layout, just the raw text output you see in your terminal would be perfect.

It would also be preferable to state (for the 1D vs 2D geometries) if the "...number of site repetitions being specified by other arguments of generate_sites." is a single integer argument or two integer arguments.

One last thing, when referring to other functions/types in the documentation (such as generate_sites), it's desirable to have it referenced so the documentation generator auto-links it to the relevant section of documentation. In our case, generate_sites should be referenced via:

[`generate_sites`](@ref)

You can see this reference behavior in action here:

"""
distance(lattice::BoundedLattice,x,y)
Returns the distance between two points in the [`BoundedLattice`](@ref).
Points `x` and `y` can be any iterable and must have the same dimensions as the [`BoundedLattice`](@ref)
(ex: `(x,y)` for a 2D lattice, `(x,y,z)` for a 3D lattice).
If the Periodic Boundary Condition option has been set to `true` for the [`BoundedLattice`](@ref),
the smallest distance between points (modulo the region) is returned, otherwise the standard Euclidean
metric is used.
```jldoctest; setup=:(using BloqadeLattices)
julia> bl = parallelepiped_region(SquareLattice(), (1,0),(0,1);) # Define 2D BoundedLattice
julia> distance(bl, (0.1, 0.1), (0.5, 1.1)) # distance between two points
1.077032961426901
julia> bl_pbc = parallelepiped_region(SquareLattice(), (1,0),(0,1);pbc=true) # Define 2D BoundedLattice with Periodic Boundary Condition
julia> distance(bl_pbc, (0.1, 0.1), (0.5, 1.1)) # distance with periodic boundary condition enabled
0.4
```
"""

@Amana-L
Copy link
Contributor

Amana-L commented Jun 12, 2023

Hi @johnzl-777, thanks for reviewing the changes!

The direction you've provided is very helpful. I'll make the additions soon.

@Amana-L
Copy link
Contributor

Amana-L commented Jun 12, 2023

Hi @johnzl-777, made another commit with the changes you suggested. Let me know if there's anything else.

Also, when running the code for RectangularLattice, it came up;
ERROR: MethodError: no method matching RectangularLattice()

So, I manually wrote the output for that one. But it's probably something that needs looking into.

Thanks.

@johnzl-777
Copy link
Collaborator Author

Hi @Amana-L ,awesome work!

Just a couple small things:

  • It would be preferable to avoid having the docstrings have a hanging indent (e.g. all the text starts at the same column, like what you see here:
    Submits a `BloqadeExpr.RydbergHamiltonian` instance to Braket with `n_shots` defining the number of times
  • The sentence structure you currently have:
Used as an argument of [`generate_sites`](@ref) function to produce tiling in a square pattern. 
    With number of site repetitions being specified by other arguments of [`generate_sites`](@ref).

would be nicer to read if the period after "pattern" wasn't there, so it reads like:

Used as an argument of [`generate_sites`](@ref) function to produce tiling in a square pattern, 
with number of site repetitions being specified by additional arguments of [`generate_sites`](@ref).

The RectangularLattice is slightly different from the other lattices in that you need to pass in a single integer on construction which ends up being an aspect ratio (essentially allowing the user to modify the length of one of the Bravais lattice vectors):

See this code in the RectangularLattice:
https://github.com/QuEraComputing/Bloqade.jl/blob/master/lib/BloqadeLattices/src/lattice.jl#L315
and
https://github.com/QuEraComputing/Bloqade.jl/blob/master/lib/BloqadeLattices/src/lattice.jl#L322

I believe the Fields section of the RectangularLattice docstring documents this but would definitely be helpful to have this in your code example!

@Amana-L
Copy link
Contributor

Amana-L commented Jun 13, 2023

Hi @johnzl-777, thanks for the suggestions! I've made the changes.

Sorry about the RectangularLattice example, I should've looked more closely. I was able to run it now and have fixed the example. Also, I added a few words about that.

I appreciate your attention to this. Let me know if there's anything else.

@johnzl-777
Copy link
Collaborator Author

Hey @Amana-L no worries! Just merged in your pull request, excellent work!

@Amana-L
Copy link
Contributor

Amana-L commented Jun 13, 2023

Hi @johnzl-777, this was fun 馃檪 thanks for your guidance along the way!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation unitaryhack
Projects
None yet
Development

No branches or pull requests

4 participants