Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #399 +/- ##
=======================================
Coverage 68.93% 68.93%
=======================================
Files 58 58
Lines 5991 5991
Branches 771 771
=======================================
Hits 4130 4130
Misses 1571 1571
Partials 290 290 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
loreloc
left a comment
There was a problem hiding this comment.
I really like this PR! I have some feedback about the videos and the text though.
-
Layer Topological Order. I think the topological ordering in the video is not correct. E.g., in the first frame, the topological ordering should consider ALL input layers. Instead, all input layers except a single one are added to the set. In other words, the topological ordering does not care about the depth, but only cares about the connectivity. Layers at different depth can be folded together at the moment. We can also link to Kahn's algorithm, which is the one being implemented https://en.wikipedia.org/wiki/Topological_sorting.
-
Torch Compiler Components. Double check the indentation of the bullet points. All bullet points are at the same level, which I think is not intended.
-
Layer Compilation Process. I really like the diagram showing the compilation process. However, it is difficult to follow on a 16:9 screen, since it is a very tall diagram. I believe it can be simplified and and rotated to be horizontal.
-
Optimization Patterns. I struggled a lot trying to understand the diagram explaining the optimization patterns (and I wrote that part! :)). I feel you can remove this diagram.
-
About the optimization and implemented optimizations. I would keep a single simplified md file explaining both the optimization and implemented optimizations. That is, instead of having the examples in a separate md file, you could use the examples to motivate and explain how an optimization is being written. E.g., you can use the Log + Softmax = LogSoftmax as an example of the "fuse" optimization. I believe this would make the md file easy to follow, without having to resort to the very abstract example being shown now (the LayerPatternOne).
-
Other suggestions. I think it would be very useful if you could put hyperlinks from the handbook README files to the documentation urls. E.g.,
TorchParametercan point to the API page. -
Minor. there are some wrong "math dollars symbols" below the "Sum Collapse Optimization" sections.
This PR add my attempt to explain the compilation process of cirkit. I had to add a dependency to "mkdocs-video" a plugin to, you guessed it, add video on the documentation pages.