📝 Getting Started With MLIR#1555
Conversation
|
@burgholzer @denialhaag @DRovara I am pretty sure everyone of you has an opinion on how this getting started guide should look - and should not look. Hence, I kindly asked for a review from each one of you. I hope this is okay! |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a new MLIR "Getting Started" guide at Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
burgholzer
added
documentation
There was a problem hiding this comment.
Thanks a lot for the work, @MatthiasReumann! All in all, it already looks pretty nice. I still tried to give some (maybe controversial) opinions, just to make sure everything is clear. None of them are really set in stone, just suggestions.
That being said though: I believe there is still some room for discussion when it comes to who the target audience should be for this "Getting Started Guide":
- Probably it shouldn't be quantum algorithm engineers who just want to compile their code. They don't care what happens in the background.
- Then, one would assume it's compiler developers. But why would compiler developers care about the
qcdialect (which the majority of this tutorial is about).
All in all, this brings me to the big question: Is it really the "correct" approach to give the getting started guide in terms of the QC dialect? People are never supposed to write MLIR code anyways - we will likely have some other front-end or DSL for that. So the only time when the code will use QC is during the translation process (and for the output, if the user does not compile down to QIR). The only advantage that QC has over QCO is that it is simpler, but it is weird to argue, for a tutorial "we won't explain the important thing because that is too complicated, so instead we explain the less important thing".
What are everyone else's thoughts on that?
burgholzer
left a comment
burgholzer
left a comment
There was a problem hiding this comment.
Thanks @MatthiasReumann for getting this started 😄
Great to see someone push this forward!
I now also gave this a thorough read.
I added some comments inline as well, but much more important than that, I would like to pick up on Damians points below.
That being said though: I believe there is still some room for discussion when it comes to who the target audience should be for this "Getting Started Guide":
Probably it shouldn't be quantum algorithm engineers who just want to compile their code. They don't care what happens in the background.
Then, one would assume it's compiler developers. But why would compiler developers care about the qc dialect (which the majority of this tutorial is about).
All in all, this brings me to the big question: Is it really the "correct" approach to give the getting started guide in terms of the QC dialect? People are never supposed to write MLIR code anyways - we will likely have some other front-end or DSL for that. So the only time when the code will use QC is during the translation process (and for the output, if the user does not compile down to QIR). The only advantage that QC has over QCO is that it is simpler, but it is weird to argue, for a tutorial "we won't explain the important thing because that is too complicated, so instead we explain the less important thing".What are everyone else's thoughts on that?
Currently, the guide feels very compact. It touches on some topics, but mostly briefly, and for the most part probably not exhaustively.
I said in one of the comments that it is "nicht fisch, nicht fleisch", and I think this also is what Damian is highlighting above. At the moment the guide itself isn't sure who it is written for.
I think this could easily be split into three parts:
- For the people knowing quantum that have never heard of MLIR; those should start with an MLIR section that explains the concepts; potentially referring back to concepts that people might know from SDKs like Qiskit or Pennylane, or from languages like OpenQASM. People knowing MLIR may skip this section.
- For the people that know classical compilers (and MLIR), but that don't know quantum too well. Those should start at a section that step by step explains quantum concepts, but tries to refer back to classical compiler and MLIR terminology wherever suitable. People knowing quantum may skip this section.
- For the people familiar with both (or that have read both previous sections). Those people, we should tell what exactly we are doing as part of the project here. This should explain the difference between the two dialects, the compilation flow, etc. This is the meat of the tutorial; the previous sections are kind of the background for the relevant crowd. Within this section, it may again be interesting to provide some anecdotes that people from one of the backgrounds would find helpful ("the program structure in QCO is very similar to DAG structures people may be familiar with from Qiskit" for example)
Overall, this should really have an educational character for people and leave as little up for imagination as possible (Damian already did a great job in the comments highlighting a couple of places where this might not yet be the case).
That's all I got for now. I hope this makes sense and helps to navigate this into the right direction. Despite the flood of comments, this is still a really great start! 🎉
| } | ||
| ``` | ||
|
|
||
| ### Interlude: MLIR Concepts |
There was a problem hiding this comment.
This is related to something that Damian brought up elsewhere: The current guide somewhat feels lacking in certain bits and pieces (in German one would say "nicht Fisch, nicht Fleisch"). It starts off with some quantum concepts, but not really to many for someone that wouldn't know much about quantum; then it switches to MLIR concepts, but directly ties these to quantum concepts, which might be confusing to the reader. I think we need to zoom out here a little bit, and find a good, coherent narrative.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
docs/mlir/GettingStarted.md (1)
83-89:⚠️ Potential issue | 🟠 MajorFill in the core tutorial sections before merge.
The two key subsections are empty (
The QC and QCO Dialects,Compilation Flow), so the guide currently misses the main learning path (QC vs QCO semantics and QC→QCO flow) promised by this PR and linked issue.✍️ Minimal structure to add now
### The QC and QCO Dialects +QC uses reference semantics and models qubits as references to allocated resources. +QCO uses value semantics with linear types, where each qubit SSA value is consumed exactly once. +Show one small side-by-side snippet (QC and QCO) for the same Bell-state step to make this concrete. ### Compilation Flow +The typical flow is: OpenQASM -> QC -> QCO -> (optional) QIR. +Add one command sequence with `mqt-cc` and briefly explain where optimizations happen. +Include one short note about current limitations (e.g., unsupported patterns), if applicable.Based on learnings from issue
#1452objectives, this guide is expected to explain QC vs QCO in detail and illustrate the QC → QCO transformation with a running example.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/mlir/GettingStarted.md` around lines 83 - 89, The two subsections "The QC and QCO Dialects" and "Compilation Flow" in GettingStarted.md are empty; fill them with content that (1) defines QC and QCO semantics (key differences, example ops/constructs and when to use each) under the "The QC and QCO Dialects" header and (2) describes the QC→QCO transformation pipeline with a small running example showing input QC IR, the transformation steps (pass names or functions) and resulting QCO IR under "Compilation Flow"; reference the section titles "The QC and QCO Dialects" and "Compilation Flow" when adding content so the guide explains QC vs QCO and demonstrates the QC→QCO flow end-to-end as requested by the linked issue.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/mlir/GettingStarted.md`:
- Line 164: The "## Conclusion" heading in the GettingStarted.md document is
empty; either remove the heading or add a brief concluding paragraph summarizing
the guide. Locate the heading text "## Conclusion" and either delete that line
(and any trailing blank lines) or append 2–4 sentences that wrap up the
document’s key takeaways and next steps so the section is not left blank.
- Around line 30-33: Fix the wording in the MLIR intro: change "The core concept
in MLIR are _dialects_" to "The core concepts in MLIR are _dialects_"; change
"floating point" to "floating-point" (e.g., "integer and floating-point
operations"); and replace "which let's us define and call functions" with "which
lets us define and call functions" (remove the apostrophe). Keep references to
SCF, arith, and Func dialects intact.
---
Duplicate comments:
In `@docs/mlir/GettingStarted.md`:
- Around line 83-89: The two subsections "The QC and QCO Dialects" and
"Compilation Flow" in GettingStarted.md are empty; fill them with content that
(1) defines QC and QCO semantics (key differences, example ops/constructs and
when to use each) under the "The QC and QCO Dialects" header and (2) describes
the QC→QCO transformation pipeline with a small running example showing input QC
IR, the transformation steps (pass names or functions) and resulting QCO IR
under "Compilation Flow"; reference the section titles "The QC and QCO Dialects"
and "Compilation Flow" when adding content so the guide explains QC vs QCO and
demonstrates the QC→QCO flow end-to-end as requested by the linked issue.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5455081e-a4a9-4d40-a96e-60c3468ccfde
⛔ Files ignored due to path filters (1)
docs/_static/mlir-regions-blocks-ops.svgis excluded by!**/*.svg
📒 Files selected for processing (2)
docs/mlir/GettingStarted.mddocs/mlir/index.md
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…-quantum-toolkit/core into docs/getting_started_mlir
…-quantum-toolkit/core into docs/getting_started_mlir
…-quantum-toolkit/core into docs/getting_started_mlir
|
@burgholzer @DRovara @denialhaag @ystade Hello, again 👋🏻 Although it isn't 100% yet (e.g. waiting on #1638 for SCF), I think another round of feedback would be great to verify if this thing is heading in the right direction. Thank you! |
There was a problem hiding this comment.
Hi @MatthiasReumann, thanks a lot for the update. I think it's already really nice, so I tried to be extra strict and pedantic to get any possible point of confusion out of the way. I do have a few further thoughts:
-
This might just be a personal preference, but I like establishing the
mqt-ccname. You, on the other hand, used the mixed form "MQT Compiler Collection (mqt-cc)" quite a lot. Personally, I would be fine with just introducing the abbreviation once and then only usingmqt-cc. One could also always use "MQT Compiler Collection". Using the mixed form so often feels a bit weird. -
While it got a lot better, I still feel like this guide suffers a bit of the "who is this for" syndrom. Some sections are important for users, other for compiler developers, others for complete newcomers. I don't think this is necessarily a problem, but maybe one could use the introduction to hint readers to specific subsections that might be interesting for them.
-
Finally, this guide still ready very much like a scientific paper (in some places, at least). That's not a huge issue, but there is a lot of "strict, stuck-up terminology" where a reader might appreciate a lighter tone. One way to fix that a bit without requiring any rephrasing might be to just use more highlighting for individual words that are important. But sometimes, I feel like sentences can also be made a bit tamer,
| The Multi-Level Intermediate Representation (MLIR) project is an extensive framework to build compilers for heterogeneous hardware. | ||
| Key to its success is the ability to represent programs at multiple levels of abstraction, as well as the capacity to lower them from higher to lower levels. | ||
|
|
||
| The core concept in MLIR is the dialect. |
There was a problem hiding this comment.
| The core concept in MLIR is the dialect. | |
| The core concept in MLIR is the *dialect*. |
Random thought: maybe we can add some more highlighting to key terms here? That makes skimming a lot easier. No strong preference for bold or italic text.
There was a problem hiding this comment.
I like that idea 👍🏻
| func.func @main() { | ||
| %lb = arith.constant 0 : index | ||
| %ub = arith.constant 100 : index | ||
| %step = arith.constant 1 : index | ||
|
|
||
| %sum_0 = arith.constant 0 : i32 | ||
| %sum = scf.for %iv = %lb to %ub step %step iter_args(%sum_iter = %sum_0) -> (i32) { | ||
| %1 = arith.index_cast %iv : index to i32 | ||
| %2 = arith.addi %sum_iter, %1 : i32 | ||
| scf.yield %2 : i32 | ||
| } | ||
| return | ||
| } |
There was a problem hiding this comment.
This example feels incredibly complex for the very first code snippet of the tutorial. In particular, the reader has to first understand how one computes a sum of numbers in a stateless setting.
I'm not saying we should cut the example but I would suggest one of the following:
- Add another example first that really just has the basics.
- Update this example so the loop does not require
iter_args.
| ``` | ||
|
|
||
| - The `func`, `arith`, and `scf` prefixes specify the dialect's name. For example, `arith.constant` represents the `constant` operation from the `arith` dialect. | ||
| - The percentage symbol `%` prefixes static single-assignment (SSA) values, a principle in which each variable is assigned exactly once and never reassigned. For instance, the first `arith.constant` operation produces the `%lb` SSA value. |
There was a problem hiding this comment.
I'm also not super convinced of only having this explanation here. I think usually, when readers check out examples, they already want to be familiar with the concepts inside it, so that they don't get hung up on some syntax while reading (which pushes them to go for a "okay the next 5 lines are that complex stuff again, let's skip them" approach).
Maybe the very basic syntax can already be explained earlier or in smaller chunks?
There was a problem hiding this comment.
Also, another pedantic thing: Why do we call them "SSA values" first? I would assume many readers are not familier with SSA, but are familiar with variables in basic programming. Would it be incorrect to instead say "The % prefixes variables that store values just like in other programming language. However, variables in MLIR are SSA" (or something like that)?
| ```console | ||
| % mqt-cc ghz.qasm --emit-qir | mlir-translate --mlir-to-llvmir > ghz.ll | ||
| ``` |
There was a problem hiding this comment.
Now that I think of it, it is kind of weird we called the option --emit-qir but then one still has to manually translate the output to QIR. I feel liek we should fix that in the compilation pipeline.
…-quantum-toolkit/core into docs/getting_started_mlir
5 participants
Description
This pull request adds a "Getting Started with MLIR" tutorial to the ReadTheDocs documentation. Thus, resolves #1452.
Checklist: