Support for for-loops #24

thisac · 2020-07-08T00:33:23Z

Context: Currently Blackbird doesn't support for-loops and thus cannot iterate through lists of statements. This would be useful when having many similar statements.

Description: Support for parsing for-loops are added to Blackbird. The user can write e.g.:

for int m in [2, 7, 3]
    MeasureFock() | m

to apply the operation to modes 2, 7 and 3, or

par array phases =
    {phase_0}
    {phase_1}
    {phase_2}
    {phase_3}
    {phase_4}

for int m in 0:5:1
    MZgate(phases[m], phases[m+1]) | [m, m+1]

to apply the MZgate with indexing from the phases list for the range 0:5:1 = [0, 1, 2, 3, 4]

Other additions:

Indexing is now supported inside both modes and arguments (e.g. MZgate(list[2], list[0]) | [list[3],list[1]]))
Expressions are supported inside all indexes (e.g. phases[-3 + 2*2]).

codecov · 2020-07-08T00:34:44Z

Codecov Report

Merging #24 into master will increase coverage by 0.13%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master      #24      +/-   ##
==========================================
+ Coverage   96.78%   96.91%   +0.13%     
==========================================
  Files          12       12              
  Lines        1615     1687      +72     
==========================================
+ Hits         1563     1635      +72     
  Misses         52       52

Impacted Files	Coverage Δ
blackbird_python/blackbird/auxiliary.py	`100.00% <100.00%> (ø)`
blackbird_python/blackbird/listener.py	`100.00% <100.00%> (ø)`
blackbird_python/blackbird/program.py	`100.00% <100.00%> (ø)`
blackbird_python/blackbird/tests/test_listener.py	`99.68% <100.00%> (+0.03%)`	⬆️

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 688c900...d6ae1e8. Read the comment docs.

josh146

This is really cool @thisac! A couple of questions:

Does for 3 iterate over [0, 1, 2] like Python, or [0, 1, 2, 3]?
Can you do for 3 -> m, to have m take the values 0, 1, 2 for each loop?
It would be good to extend this to support not just passing the number of iterations, but providing the start, end, and step values. E.g., for 6, 20, 2 -> m

blackbird_python/blackbird/listener.py

josh146 · 2020-07-08T11:02:38Z

blackbird_python/blackbird/listener.py

+            if self._infor:
+                return


Might be worth commenting to show what is happening here 🤔

blackbird_python/blackbird/listener.py

thisac · 2020-07-08T22:35:09Z

Does for 3 iterate over [0, 1, 2] like Python, or [0, 1, 2, 3]?

It should iterate over [0, 1, 2], since it creates a range(3) sequence.

Can you do for 3 -> m, to have m take the values 0, 1, 2 for each loop?

Yes, that's how it currently works.

It would be good to extend this to support not just passing the number of iterations, but providing the start, end, and step values. E.g., for 6, 20, 2 -> m

Just implemented it with the syntax for 6:20:2 -> m. What do you think?

I updated the for-loop to support the same type of lists as the modes: [1, 2, 3], (1, 2, 3) or 1, 2, 3, and I think that the last option, without any brackets/parentheses might work best with the rest of the syntax (although that's debatable, and if you prefer to fix it to [] I'll do that). I also added support for lists that contain both parameters (e.g. {phase_0} or other types), and they are callable from within any statement, including for-loops. So, this should now work (although I haven't written any thorough tests yet):

name test_name
version 1.0
target fock (num_subsystems=1, cutoff_dim=7, shots=10)

par array phases =
    {phase_0}
    {phase_1}
    {phase_2}
    {phase_3}
    {phase_4}

float alpha = 0.3423
Coherent(alpha, sqrt(pi)) | 0
for 0, 3, 1 ->  int m
    MZgate(phases[m], phases[m+1]) | [m, m+1]

for 2:20:2 ->  int m
    MeasureFock() | m

Also, any type of array will currently be considered a list, by flattening it before attempting to extract a value from it. So this should work in the context above as well:

par array phases =
    {phase_0}, {phase_1}
    {phase_2}, {phase_3}
    {phase_4}

co9olguy · 2020-07-08T22:42:08Z

Just curious about the philosophy for the chosen semantics here, as I found both
for 3
and
for [2, 7, 3] -> int m
to be unintuitive

Is this something that is forced by our choice of lexer/parser?

Historical note: originally, blackbird was meant to be kept as close to "python" as possible, since the way a user declares their program in SF is essentially also "blackbird code" (with some small, unavoidable, modifications between "standalone blackbird" and "python-embedded blackbird"). Why would we want to do that here, where---at least to my naive eye---it doesn't seem necessary to depart from python style?

thisac · 2020-07-09T00:38:07Z

Just curious about the philosophy for the chosen semantics here, as I found both
for 3
and
for [2, 7, 3] -> int m
to be unintuitive

Is this something that is forced by our choice of lexer/parser?

Historical note: originally, blackbird was meant to be kept as close to "python" as possible, since the way a user declares their program in SF is essentially also "blackbird code" (with some small, unavoidable, modifications between "standalone blackbird" and "python-embedded blackbird"). Why would we want to do that here, where---at least to my naive eye---it doesn't seem necessary to depart from python style?

@co9olguy There's no particular reason for choosing this style at all. I just played around with different semantics during the hackathon, but I could easily change it to be more python-like if that's preferred. 🙂

I chose to go with this since I thought it looked nice, and vaguely pythonic, while still following the style that I thought Blackbird had (e.g. few : and brackets/parentheses). Same with the array declarations, that were already there but that I simply expanded to include parameters (e.g. {phase_0}) and indexing. The code in my last comment above shows better what I envisioned, but I have no issues with changing it to something else, or simply just making it look like python instead.

josh146 · 2020-07-09T04:11:52Z

I'm in favour of making the syntax more Pythonic, with the caveat that Python doesn't actually have a native for loop with counter; instead, the for construct in Python is really a combination of a foreach loop and generator functions like range().

Implementing a native for loop would be much simpler than always requiring a foreach loop and iterator object.

We could:

Implement separate for and foreach keyword

foreach int m in [0, 3, 1]
    MZgate(phases[m], phases[m+1]) | [m, m+1]

for int m = 0,10,2
    MeasureFock() | m

Use the same keyword for for and foreach, but distinguish the two based on another syntactic pattern:
```
for int m in [0, 3, 1, 4]
    MZgate(phases[m], phases[m+1]) | [m, m+1]

for int m = 0:10:2  # could also use 'in' here?
    MeasureFock() | m
```
(e.g., in this case the syntax defining the loop counter m distinguishes the behaviour)

I quite like the second option. 0:10:2 reminds me of Python's slicing syntax, is a common syntax in various languages to distinguish a range, and avoids the pitfall of having a separate range() function, that will be a lot harder to implement and have a lot of edge cases.

That being said, I do quite like @thisac's chosen syntax, it 'looks cool' 😆

thisac · 2020-07-21T19:04:51Z

[ch260]

thisac · 2020-07-24T19:27:19Z

@josh146 @co9olguy I've changed the syntax for the for-loops to be more python-like. E.g.

for int m in [2, 7, 3]
    MeasureFock() | m

to apply the operation to modes 2, 7 and 3, or

par array phases =
    {phase_0}
    {phase_1}
    {phase_2}
    {phase_3}
    {phase_4}

for int m in 0:5:1
    MZgate(phases[m], phases[m+1]) | [m, m+1]

to apply the MZgate with indexing from the phases list for the range 0:5:1 = [0, 1, 2, 3, 4].

I also added some tests and fixed some issues that I found along the way. 🙂

josh146

Looking great @thisac! My main question was with respect to indexing of different shaped arrays, but apart from that it's looking good

blackbird_python/blackbird/listener.py

blackbird_python/blackbird/tests/test_listener.py

josh146 · 2020-07-27T05:39:33Z

blackbird_python/blackbird/tests/test_listener.py

+    def test_var_idx_in_modes(self, parse_input_mocked_metadata):
+        """Test that array indexing works inside modes"""
+        bb = parse_input_mocked_metadata(
+            "int array vars =\n\t1, 2\n\nMZgate(0, 1) | [vars[0], vars[1]]"
+        )
+        assert bb.operations == [
+            {'op': 'MZgate', 'args': [0, 1], 'kwargs': {}, 'modes': [1, 2]}
+        ]


This test tests the case

int array vars = 1, 2

Is it worth also testing the case

int array vars = 1 2

(does this affect how you index?)

What about something like

int array vars = 1, 2 3, 4

I deliberately made the arrays always behave as flat, in the sense that you can write them (and store them) as nested numpy-arrays but they will behave as flat with regards to indexing. So your examples above would basically behave the same only with the last one having two more elements.

I thought it looked nice to be able to list parameters vertically rather than horizontally,

par array phases = {phase_0} {phase_1} {phase_2} {phase_3} {phase_4}

and vice versa with regards to values.

int array vars = 1, 2, 3, 4, 5, 42, 6

blackbird_python/blackbird/tests/test_listener.py

josh146 · 2020-07-27T05:42:26Z

blackbird_python/blackbird/tests/test_listener.py

+        with pytest.raises(ValueError, match="invalid value"):
+            bb = parse_input_mocked_metadata(
+                "for int m in [1, 4.2, 9]\n\tMZgate(0, 1) | [0, 1]"
+            )


Great tests! Might also be worth testing

Indexing different shaped arrays (e.g., column vectors, matrices)

The case where stepsize is ignored, if this is allowed (wasn't sure based on the tests above)

Having expressions in the for loop syntax?

Added tests for everything you mentioned. Realised that

int array vars = 1, 2 3, 4

works, while

int array vars = 1, 2 3

does not; but I think this is a minor issue and can be looked into closer if/when we decide to rework the BB array type to e.g. include 2d indexes.

I think that should actually be fine, I'm not sure

int array vars = 1, 2 3

is intended to/should be allowed. Since this corresponds to a 'ragged' numpy array

josh146

🔥

josh146 · 2020-07-29T04:07:31Z

Something that just crossed my mind --- we will need to update the documentation, and especially the syntax.rst page, to take into account these changes!

In particular, to ensure that any deviations from Python are clearly laid out, such as flat indexing and for loops.

thisac added 2 commits July 7, 2020 20:25

Add for-loop

e8c70bc

Fix kwarg tests

c6ca4ff

thisac requested a review from josh146 July 8, 2020 00:33

thisac self-assigned this Jul 8, 2020

thisac added the enhancement New feature or request label Jul 8, 2020

thisac marked this pull request as draft July 8, 2020 00:35

josh146 reviewed Jul 8, 2020

View reviewed changes

thisac added 5 commits July 8, 2020 15:05

Add support for expressions in for

9d92321

Change var name to _in_for

59b9e73

Add support for parameter lists

c9d8d71

Update example test-script

785bcaf

Add support for start, stop, step ranges

ac654c1

thisac added 10 commits July 24, 2020 11:48

Pythonise for-loop

69b92be

Update blackbird files

9e1c132

Update for-loop in listener

794fdcb

Remove f-strings

6942604

Fix expression in modes

2fa49be

Add tests for for-loops and indexing

667ad77

Fix mode bug

0a2e610

Add error raise test

a45f217

Add for-loop error test

824b98d

Add clear vars at enterProgram

d33d640

thisac marked this pull request as ready for review July 24, 2020 19:24

thisac requested a review from josh146 July 24, 2020 19:24

Remove string-support in modes

22f7131

thisac changed the title ~~[WIP] Support for for-loops~~ Support for for-loops Jul 24, 2020

Fix coverage

20ff5bf

josh146 reviewed Jul 27, 2020

View reviewed changes

thisac added 3 commits July 28, 2020 13:28

Add tests

3b3ddfb

Add error type

c7565ff

Add array test

d6ae1e8

thisac requested a review from josh146 July 28, 2020 19:11

josh146 approved these changes Jul 29, 2020

View reviewed changes

thisac merged commit 60e1ae4 into master Jul 29, 2020

thisac deleted the for_loop branch July 29, 2020 14:08

thisac mentioned this pull request Aug 13, 2020

Add for-loops to documentation #28

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for for-loops #24

Support for for-loops #24

thisac commented Jul 8, 2020 •

edited

Loading

codecov bot commented Jul 8, 2020 •

edited

Loading

josh146 left a comment

josh146 Jul 8, 2020

thisac commented Jul 8, 2020 •

edited

Loading

co9olguy commented Jul 8, 2020 •

edited

Loading

thisac commented Jul 9, 2020

josh146 commented Jul 9, 2020 •

edited

Loading

thisac commented Jul 21, 2020

thisac commented Jul 24, 2020

josh146 left a comment

josh146 Jul 27, 2020

thisac Jul 28, 2020

thisac Jul 28, 2020 •

edited

Loading

josh146 Jul 29, 2020

josh146 Jul 27, 2020

thisac Jul 28, 2020

josh146 Jul 29, 2020

josh146 left a comment

josh146 commented Jul 29, 2020

Support for for-loops #24

Support for for-loops #24

Conversation

thisac commented Jul 8, 2020 • edited Loading

codecov bot commented Jul 8, 2020 • edited Loading

Codecov Report

josh146 left a comment

Choose a reason for hiding this comment

josh146 Jul 8, 2020

Choose a reason for hiding this comment

thisac commented Jul 8, 2020 • edited Loading

co9olguy commented Jul 8, 2020 • edited Loading

thisac commented Jul 9, 2020

josh146 commented Jul 9, 2020 • edited Loading

thisac commented Jul 21, 2020

thisac commented Jul 24, 2020

josh146 left a comment

Choose a reason for hiding this comment

josh146 Jul 27, 2020

Choose a reason for hiding this comment

thisac Jul 28, 2020

Choose a reason for hiding this comment

thisac Jul 28, 2020 • edited Loading

Choose a reason for hiding this comment

josh146 Jul 29, 2020

Choose a reason for hiding this comment

josh146 Jul 27, 2020

Choose a reason for hiding this comment

thisac Jul 28, 2020

Choose a reason for hiding this comment

josh146 Jul 29, 2020

Choose a reason for hiding this comment

josh146 left a comment

Choose a reason for hiding this comment

josh146 commented Jul 29, 2020

thisac commented Jul 8, 2020 •

edited

Loading

codecov bot commented Jul 8, 2020 •

edited

Loading

thisac commented Jul 8, 2020 •

edited

Loading

co9olguy commented Jul 8, 2020 •

edited

Loading

josh146 commented Jul 9, 2020 •

edited

Loading

thisac Jul 28, 2020 •

edited

Loading