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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add for-loops to documentation #28

Merged
merged 12 commits into from
Nov 2, 2020
Merged

Conversation

thisac
Copy link
Contributor

@thisac thisac commented Aug 13, 2020

This PR updates the Blackbird documentation by:

  • Adding a section about for-loops (Support for for-loops #24)
  • Updating ANTLR4 version to 4.8 in install instructions
  • Mentioning that keyword arguments are accepted inside operations (as well as using it inside an example)
  • Adding a paragraph on array indexing
  • Adding a paragraph on the type keyword metadata

@thisac thisac requested a review from josh146 August 13, 2020 22:59
@codecov
Copy link

codecov bot commented Aug 13, 2020

Codecov Report

Merging #28 into master will increase coverage by 0.66%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master      #28      +/-   ##
==========================================
+ Coverage   96.94%   97.61%   +0.66%     
==========================================
  Files          12       12              
  Lines        1704     2141     +437     
==========================================
+ Hits         1652     2090     +438     
+ Misses         52       51       -1     
Impacted Files Coverage Δ
blackbird_python/blackbird/program.py 100.00% <0.00%> (ø)
blackbird_python/blackbird/listener.py 100.00% <0.00%> (ø)
blackbird_python/blackbird/auxiliary.py 100.00% <0.00%> (ø)
blackbird_python/blackbird/tests/test_program.py 100.00% <0.00%> (ø)
blackbird_python/blackbird/tests/test_auxiliary.py 100.00% <0.00%> (ø)
blackbird_python/blackbird/tests/test_listener.py 100.00% <0.00%> (+0.30%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fccaab1...8664a9e. Read the comment docs.

@thisac thisac self-assigned this Aug 13, 2020
Copy link
Member

@josh146 josh146 left a comment

Choose a reason for hiding this comment

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

Thanks @thisac! Just a question on the par type, as well as some additional questions regarding for loops (that only just came to me reading through the examples)

doc/installing.rst Show resolved Hide resolved
doc/syntax.rst Show resolved Hide resolved
doc/syntax.rst Outdated Show resolved Hide resolved
doc/syntax.rst Show resolved Hide resolved
doc/syntax.rst Outdated Show resolved Hide resolved
doc/syntax.rst Show resolved Hide resolved
doc/syntax.rst Outdated Show resolved Hide resolved
doc/syntax.rst Show resolved Hide resolved
doc/syntax.rst Show resolved Hide resolved
@thisac thisac requested a review from josh146 October 19, 2020 22:33
@thisac
Copy link
Contributor Author

thisac commented Oct 19, 2020

@josh146 Updated the docs with some new information about the program type, and fixed the issues that we discussed some time ago. I still need to update Blackbird to support arrays being free parameters, although I'm not sure if it's necessary to add any comments about this in the docs.

josh146
josh146 previously approved these changes Oct 21, 2020
Copy link
Member

@josh146 josh146 left a comment

Choose a reason for hiding this comment

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

Just read through it; reads clearly and succinctly. Thanks @thisac

doc/syntax.rst Outdated Show resolved Hide resolved
Co-authored-by: Josh Izaac <josh146@gmail.com>
@thisac thisac requested a review from josh146 November 2, 2020 15:29
@thisac
Copy link
Contributor Author

thisac commented Nov 2, 2020

@josh146 Added a part on the TDM program as well, attempting to briefly explain how the TDM program type differs; mainly trying to explain the use of the p-type keywords. Let me know what you think! 🙂

josh146
josh146 previously approved these changes Nov 2, 2020
Copy link
Member

@josh146 josh146 left a comment

Choose a reason for hiding this comment

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

Looks good @thisac, just have a question regarding p arrays (it's hard to tell if they are required/what happens if you include one that isn't).

I suppose the distinction needs to be made:

  • If an array starts with p, each element in the array corresponds to a single timebin
  • If an array doesn't start with p, the entire array is passed to the gate in every timebin (it is a constant across timebins)

Now that I've written it out... it does feel a bit like a separate keyword, akin to const in C++ 😆

loop int array bs = 
    1, 2

doc/syntax.rst Outdated Show resolved Hide resolved
doc/syntax.rst Outdated
Comment on lines 466 to 468
TDM programs has reserved words starting with a ``p`` followed by a number; e.g.,
``p0``, ``p1``, or ``p42``. These are placeholders for the parameters in their
corresponding arrays (see script example below).
Copy link
Member

Choose a reason for hiding this comment

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

What happens if you don't use p? E.g.,

    name tdm
    version 1.0
    type tdm (temporal_modes=42, copies=3)

    int array a =
        1, 2

    int array b =
        3, 4

    Sgate(0.7, 0) | 1
    BSgate(a, 0.0) | [0, 1]
    MeasureHomodyne(phi=b) | 0

Will that produce a parsing error?

Copy link
Contributor Author

@thisac thisac Nov 2, 2020

Choose a reason for hiding this comment

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

Yes, it should. In that case I believe it would try to validate the type of a and b and fail (since they are arrays).

I was wrong above. This does work, but will insert the arrays directly into the operations inside the Blackbird program.

@thisac
Copy link
Contributor Author

thisac commented Nov 2, 2020

Thanks @josh146,

  • If an array starts with p, each element in the array corresponds to a single timebin

Yes, this would be the general use case. They're not strictly required, only used when wanting to pass different values to the different time-bins. Do you think this needs clarification still?

  • If an array doesn't start with p, the entire array is passed to the gate in every timebin (it is a constant across timebins)

This wouldn't (and shouldn't) work, right? Blackbird doesn't support passing an array to a gate. But, assuming that gates could receive arrays as parameters, then yes, this would be the case.

@josh146
Copy link
Member

josh146 commented Nov 2, 2020

This wouldn't (and shouldn't) work, right? Blackbird doesn't support passing an array to a gate. But, assuming that gates could receive arrays as parameters, then yes, this would be the case.

This should work, as you say, for gates that accept arrays (e.g., Interferometer, which is supported by Blackbird):

name tdm
version 1.0
type tdm (temporal_modes=42, copies=3)

int array U =
    1, 0
    0, 1

int array p0 =
    3, 4

Sgate(0.7, 0) | 1
Interferometer(U) | [0, 1] # All of U with every timebin 
MeasureHomodyne(phi=p0) | 0  # one element of p0 per timebin

josh146
josh146 previously approved these changes Nov 2, 2020
Co-authored-by: Josh Izaac <josh146@gmail.com>
@thisac thisac merged commit 50364f3 into master Nov 2, 2020
@thisac thisac deleted the ch326-blackbird-documentation branch November 2, 2020 17:15
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.

2 participants