Standardize docs across nx, exla, torchx#1770
Conversation
|
Apologies but I am not a fan of this approach. We are introducing a bunch of modules, which have a runtime cost, to document APIs that do not really exist and people should not invoke. Otherwise this will appear as suggestions on LSP, ExDoc autocompletes, etc. We should figure out how to structure this using regular pages/guides as much as possible. |
I think we can keep the same general idea where we have a standardized nested structure that corresponds to the Nx modules (e.g. EXLA > Backend Documentation > Nx > LinAlg) which is written as Markdown, Livemd or cheatsheets, as relevant. The most important part of this PR for me is having an easy way to look things up structurally. I suspect livebooks will provide the closest experience to regular modules, and cheatsheets can be complimentary documentation to that. We might also want some sort of CI check to ensure the documentation gets updated if we add or remove functions to the original Nx modules, but we can leave this as a follow-up. |
|
I think you can pass files to doctest to make sure their content is correct. But that means using .cheatmd or .md files. Any should be fine! |
38b13b5 to
05d66a1
Compare
|
|
||
| These guides describe how EXLA lowers and executes Nx operations. They mirror | ||
| the structure of the Nx API (see the [backend documentation convention](https://hexdocs.pm/nx/backend_documentation-convention.html)) | ||
| but are **not callable** — use `Nx` and `compiler: EXLA` in your code. |
There was a problem hiding this comment.
This is wrong now, as it is only markdown files.
| ## take/3 | ||
|
|
||
| Tensor indexing (`%Nx.Block.Take{}`). | ||
|
|
||
| ### Lowering | ||
|
|
||
| Lowered to StableHLO `gather` in `EXLA.Defn` (not a custom call). | ||
|
|
||
| ### Options | ||
|
|
||
| * `:axis` — honoured; indices are gathered along this axis | ||
|
|
||
| ### Platforms | ||
|
|
||
| All EXLA clients. |
There was a problem hiding this comment.
Question: what documenting this particular behaviour is giving us? What it becomes in StableHLO is not necessarily relevant. If it is, we will probably benefit more from having a debug_stablehlo option that prints it.
There was a problem hiding this comment.
We do have this in EXLA.to_mlir_module
There was a problem hiding this comment.
Now to think out loud about lowering patterns, this function is implemented as an Nx.Block, so. users might want to override the imolementation.
Though at that point they might as well look at the source code.
Not to decide anything, but maybe. what we want here is to scope documenting divergent behavior and backend specific options only?
There was a problem hiding this comment.
Divergent behavior or specific trade-offs. Yes. Most of what EXLA does is already abstracted several layers away, saying we use gather won’t add much. But saying function X does not support A option or has a special flag can help. The question is what users of this library may want to know. If you are implementing custom blocks, then the source is already the best source of truth anyway.
| ], | ||
| "guides/backend_documentation/nx_lin_alg.md": [ | ||
| filename: "backend_documentation-nx_lin_alg", | ||
| title: "Nx.LinAlg (EXLA)" |
There was a problem hiding this comment.
This is already the title, do we need to give it again? Same above.
polvalente
left a comment
There was a problem hiding this comment.
Overall this looks great, I just think we need to clean up some of the things I pointed out
|
|
||
| ## Backend notes | ||
|
|
||
| See [Torchx](https://hexdocs.pm/torchx/backend_documentation-nx_lin_alg.html#cholesky-1). |
There was a problem hiding this comment.
I don't think we should reference backends from Nx itself
Co-authored-by: Paulo Valente <16843419+polvalente@users.noreply.github.com>
Closes #1759