improve mlir & mlir_opt property documentation#2975
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2975 +/- ##
=======================================
Coverage 96.97% 96.97%
=======================================
Files 166 166
Lines 19209 19209
Branches 1788 1788
=======================================
Hits 18628 18628
Misses 429 429
Partials 152 152 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
isaacdevlugt
left a comment
isaacdevlugt
left a comment
There was a problem hiding this comment.
Just needs a changelog entry :heh:
Otherwise, nice 🚀
Co-authored-by: Isaac De Vlugt <34751083+isaacdevlugt@users.noreply.github.com>
|
|
||
| Returns: | ||
| str: The textual MLIR module after applying user passes and the default pipeline. | ||
| None: If compilation has not yet occurred. |
There was a problem hiding this comment.
This statement is a bit tricky. Technically, accessing this property triggers the "compilation" (at least compilation in the MLIR / core Catalyst compiler sense), but what needed to have happened before is that the capture and generate_ir stages of the QJIT object needed to have been run, whether via AOT compilation, JIT compilation, or some other way.
There was a problem hiding this comment.
Good point! It's kind of difficult to convey all of that concisely, what about something like None: If MLIR cannot be generated for the circuit. and we can explain further in the core of the docstring?
There was a problem hiding this comment.
Yeah that makes sense! Maybe a slight modification, since the MLIR having been generated is the criteria, and our programs are more general than circuits:
None: If the MLIR has not yet been generated for this program
There was a problem hiding this comment.
If optimized MLIR has not yet been generated for the program.
The condition for both properties is the same actually, because they take the jax-generated mlir (.mlir_module) as input to produce their output.
There was a problem hiding this comment.
I guess I was trying to get at the required success of running the requested pipeline to generate .mlir_opt, whereas .mlir_module is sufficient for .mlir.
There was a problem hiding this comment.
If there is a failure in the pipeline an error should be raised. The None result is produced precisely when the input IR isn't available.
3 participants
Context:
The
QJIT.mlirproperty is relatively poorly documented. The return type is unclear and unspecified, and the docstring provides little clarification.Description of the Change:
Adds type hints and updates docstring.
Benefits:
clearer
mlirandmlir_optproperties.Possible Drawbacks:
Related GitHub Issues:
[sc-123089]