-
Notifications
You must be signed in to change notification settings - Fork 17
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
feat(docs): add docs for list comprehension #173
Conversation
b33e898
to
ab8a2d7
Compare
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.
Thanks @tohrnii, looks good overall, and the examples are great! I added a few minor comments inline. Regarding the deployment of the docs - I think it's ok to keep deploying the next
docs for now. Maybe we should change that at the 0.3 release
|
||
## List Comprehension | ||
|
||
List comprehension provides syntactic convenience for creating new vectors. It is similar to the list comprehension syntax in Python. The following examples show how to use List Comprehension in AirScript. |
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.
nits:
- I would change the first sentence to "List comprehension provides a simple way to create new vectors."
- I would uncapitalize "list comprehension" in the last sentence and anywhere else in this file
|
||
To make writing constraints easier, AirScript provides a number of syntactic conveniences. These are described in this section. | ||
|
||
## List Comprehension |
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: I would make this "List comprehension" for consistency
|
||
To make writing constraints easier, AirScript provides a number of syntactic conveniences. These are described in this section. | ||
|
||
## List Comprehension |
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: I would make this "List comprehension" for consistency
``` | ||
let x = [a * 2 for a in b] | ||
``` | ||
This will create a new vector with the same length as `b` and each element will be twice the corresponding element in `b`. |
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: I would probably say "...and the value of each element will be twice that of the corresponding element in b
."
``` | ||
let x = [2^i * a for (i, a) in (0..5, b)] | ||
``` | ||
Ranges can also be used as iterables. This will create a new vector with length 5 and each element will be the corresponding element in `b` multiplied by 2 raised to the power of the index of the element. This will throw an error if `b` is not of length 5. |
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: maybe we can rephrase this slightly for clarity about the index. Something like "Ranges can also be used as iterables, which makes it easy to refer to an element and its index at the same time. This will create a new vector ... multiplied by 2 raised to the power of the element's index. This will throw an error..."
let z = sum([a * 2 for a in a]) | ||
``` | ||
|
||
where x and y represent the sum of all trace column values in the trace columns group `a` and z represents the sum of all trace column values in the trace columns group `a` multiplied by 2. |
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: let's make this a sentence, add backticks, and switch to using the "binding" terminology:
"In the above, x
and y
both represent the sum of all trace column values in the trace column binding a
. z
represents the sum of all trace column values in the trace column binding a
multiplied by 2
."
let z = prod([a + 2 for a in a]) | ||
``` | ||
|
||
where x and y represent the product of all trace column values in the trace columns group `a` and z represents the product of all trace column values in the trace columns group `a` added by 2. |
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 you make the same adjustments here as above? (sentence, backticks, binding terminology)
docs/src/description/keywords.md
Outdated
@@ -11,8 +11,10 @@ AirScript defines the following keywords: | |||
- `integrity_constraints`: used to declare the [source section](./structure.md#source-sections) where the [integrity constraints are described](./constraints.md#integrity_constraints). | |||
- `let`: used to declare intermediate variables in the boundary_constraints or integrity_constraints source sections. | |||
- `periodic_columns`: used to declare the [source section](./structure.md#source-sections) where the [periodic columns are declared](./declarations.md). _They may only be referenced when defining integrity constraints._ | |||
- `prod`: used to declare list folding to multiply a list of values. |
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: I would probably adjust this to "Used to fold a list into a single value by multiplying all of the values in the list together."
docs/src/description/keywords.md
Outdated
- `public_inputs`: used to declare the [source section](./structure.md#source-sections) where the [public inputs are declared](./declarations.md). _They may only be referenced when defining boundary constraints._ | ||
- `random_values`: used to declare the [source section](./structure.md#source-sections) where the [random values are described](./declarations.md). | ||
- `sum`: used to declare list folding to sum a list of values. |
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: I would probably adjust this to "Used to fold a list into a single value by summing all of the values in the list."
docs/src/introduction.md
Outdated
The language also includes support for other syntactic sugar like list comprehension and list folding to make writing constraints easier. | ||
(e.g. `let x = [k * c for (k, c) in (k, c[1..4])]`, `let y = sum([k * c for (k, c) in (k, c[1..4])])`) | ||
|
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: I would probably say "The language also includes some convenience syntax like list comprehension and ..."
Thank you @grjte for the review. Made suggested changes and corrections. |
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.
Looks good @tohrnii! I left a comment inline about one typo. Can you fix it before merging?
``` | ||
let x = [2^i * a for (i, a) in (0..5, b)] | ||
``` | ||
Ranges can also be used as iterables, which makes it easy to refer to an element and its index at the same time. This will create a new vector with length 5 and each element will be the corresponding element in `b` multiplied by 2 aised to the power of the element's index. This will throw an error if `b` is not of length 5. |
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: there's a typo in "raised"
This PR adds docs for list comprehension and list folding. This PR should be merged after #136 and #154.
Question: Should we change the deployed docs branch from
next
tomain
now?