📝 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! |
📝 WalkthroughWalkthroughAdds 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✨ Simplify code
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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! 🎉
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
denialhaag
left a comment
denialhaag
left a comment
There was a problem hiding this comment.
Thanks for addressing my previous round of comments, @MatthiasReumann!
Apart from #1555 (comment), this LGTM now. In an effort to get this along, I'll already approve this PR now. 😌
ystade
left a comment
ystade
left a comment
There was a problem hiding this comment.
@MatthiasReumann I am so sorry that I let you wait for so long. There had been quite some very important and timely tasks before. However, that does not diminish your great effort. I think we are close to the finish line. I left few very minor comments inline. Not sure whether all reviewers have to finish their review. For me it would be fine to merge this after all comments are resolved. However, I leave the final decision to @burgholzer .
Once again, a big thank you to @MatthiasReumann !
| ```cpp | ||
| // file: mlir/lib/Dialect/QCO/Transforms/Optimizations/CancelConsecutiveHadamards.cpp | ||
|
|
||
| #include "mlir/Dialect/QCO/IR/QCOOps.h" // Newly added. |
There was a problem hiding this comment.
I do not get the "Newly added". What does it refer to?
There was a problem hiding this comment.
A snippet above uses the same file. With "Newly added" I tried to convey the idea that this include must be added to the above snippet. Because you didn't get that, I don't think I managed to do this successfully. Any ideas on how to make this more clear?
| ```console | ||
| % mqt-cc ghz.qasm --emit-qir | mlir-translate --mlir-to-llvmir > ghz.ll | ||
| ``` |
|
I'd like to take one final look through the guide once all feedback points have been addressed. |
Co-authored-by: Yannick Stade <100073938+ystade@users.noreply.github.com> Signed-off-by: matthias <matthias@bereumann.com>
…-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: