-
Notifications
You must be signed in to change notification settings - Fork 83
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
documentation: do a pass over MLIR IR notebook #3875
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3875 +/- ##
=======================================
Coverage 91.26% 91.26%
=======================================
Files 466 466
Lines 57905 57905
Branches 5570 5570
=======================================
Hits 52845 52845
Misses 3634 3634
Partials 1426 1426 ☔ View full report in Codecov by Sentry. |
docs/marimo/mlir_ir.py
Outdated
mo.md( | ||
r""" | ||
We'll look at it more in detail, but let's take a look at some broad properties: | ||
|
||
1. The IR is "structured". | ||
|
||
There are curly braces (`{{}}`) with indented code in them, a bit like the C family languages. | ||
|
||
2. There are assignments with `=` | ||
|
||
One important detail is that each value is assigned to exactly once. | ||
MLIR IR is in [SSA form](https://en.wikipedia.org/wiki/Static_single-assignment_form), a property that makes it easier to determine the contents of a value when reasoning about code. | ||
|
||
3. The things immediately to the right of the assignments are in the form `first.second` | ||
|
||
These things are the names of operations. | ||
These operations are the core building blocks of MLIR IR, and their structure and meaning is indicated by this name. | ||
The names are all in two parts, where the first part is the name of a dialect, a kind of namespace for related concepts, and the second makes the operation name unique within the dialect. | ||
|
||
With this in mind, let's zoom in to the first operation. | ||
""" | ||
) | ||
mo.md(r"""This notebook explains all the components of the above snippet of code.""") |
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.
Why removing this? This is quite useful outside of workshops
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.
The broad feedback was that the wall of text is not a great start to the notebook. I'll intersperse these in the relevant parts below.
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 also have the following suggestions for this notebook:
- "MLIR and xDSL use an encoding of the IR as a textual format for debugging, testing, and storing intermediate representations of programs."
to
"MLIR and xDSL use a textual encoding of the IR for debugging, testing, and storing intermediate representations of programs."
Otherwise it is a bit too close to the "assembly textual format" and at the end of the way formatting is different than textual vs bytecode representation.
- "The func dialect contains building blocks to model functions and function calls."
to
"The func dialect contains building blocks to model function definitions and their calls."
- "The above function takes two 32-bit integers, and returns them in the opposite order "
Missing a period at the end of this sentence.
-
The cell "Unhide the cell below to see the solution:" is not visible for me? I don't get an option to unhide anything. Same for all appearances of this in the notebook.
-
"Instead, the loop body takes in some number of immutable values and yields the same number of values to use for the next loop. After all the iterations, these values are returned by the operation."
to
"Instead, the loop body takes in a number of immutable values and yields the same number of values to use for the next iteration. When all iterations are complete, these values are returned by the operation."
docs/marimo/mlir_ir.py
Outdated
@@ -44,7 +44,7 @@ def _(mo, triangle_text): | |||
|
|||
@app.cell(hide_code=True) | |||
def _(mo): | |||
mo.md(r"""This notebook explains all the components of the above snippet of code.""") | |||
mo.md(r"""This notebook explains all the components of the above snippet of code. The sections below are structured by _dialect_, a kind of namespace for related constructs.""") |
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.
mo.md(r"""This notebook explains all the components of the above snippet of code. The sections below are structured by _dialect_, a kind of namespace for related constructs.""") | |
mo.md(r"""This notebook explains all the components of the above snippet of code. The sections below are structured by _dialect_, a namespace for related constructs.""") |
No description provided.