m-labs / artiq

A leading-edge control system for quantum information experiments
https://m-labs.hk/artiq
GNU Lesser General Public License v3.0
422 stars 194 forks source link

Developer documentation #867

Open hartytp opened 6 years ago

hartytp commented 6 years ago

Are there any plans to improve the coverage of documentation for ARTIQ/misoc/migen?

The ARTIQ user-facing documentation is generally pretty good. But, as soon as one wants to look at how things are implemented, the documentation becomes sparse to nonexistent. Currently, understanding how things are implemented in ARTIQ means reading and properly understanding the sources of large chunks of ARTIQ+migen+misoc. Short (often single-letter) function and variable names also don't help users to get up to speed!

It would be great to have both high-level docs explaining how things fit together, as well as brief explanations of how/when to use common components (e.g. misoc.interconect.stream.Endpoint).

jbqubit commented 6 years ago

I agree. The level of documentation of misoc and migen was a hurdle when Arpit was working on a hardware driver for Zotino. Some ad hoc breadcrumb trails I found helpful including the following.

https://www.wdj-consulting.com/blog/migen-port.html

https://lab.whitequark.org/notes/2016-10-18/implementing-an-uart-in-verilog-and-migen/

Zotino example code https://github.com/m-labs/artiq/pull/842

sbourdeauducq commented 6 years ago

Short (often single-letter) function and variable names

Where?

hartytp commented 6 years ago

@sbourdeauducq All over the place. Lots of variable names like i, o, a, b, c, m, j, k etc and functions with names like spline.tri.

My general feeling is that once one understands the code, what it does, how it fits in with other code, and how it's intended to be used then it's all clean and makes sense. But, to get to that point from scratch one has to read the code a few times and then search through various M-Labs repositories to find instances of it being used.

e.g. the misoc stream stuff is a really nice solution to a common problem. But, to understand it, one has to understand what problem it's solving and, without any docstrings, that requires some detective work.

jordens commented 6 years ago

A lot of those single letter variables follow just follow signal names that are as old as the flip flop itself. You have to address that critique to the inventors of the flip-flop and everybody else after them. I see no reason to comment that. Ijk have been indices in programming languages and math for how long? That's not going to change and the don't need comments.

jordens commented 6 years ago

Migen and misoc do need more documentation. I think we already have issues filed there. Artiq could profit from documentation in a few areas. E.g. writing an rtio driver. I think there is an issue about that as well.

dhslichter commented 6 years ago

It would also be useful to have documentation covering the architecture, both in overview and medium level detail, of how all the various processes are spawned, communicate, etc within ARTIQ -- workers, master, applets, compiler, etc. This is not something most users should touch or care about, but there are instances when I definitely wish I knew a bit more of what was going on under the hood when trying to optimize how something is done, or hack on a feature I want. Writing up this kind of documentation would be a line item in a new contract, clearly.

jordens commented 6 years ago

Some of this would also hopefully make its way into the paper someday... Maybe I'll churn out some of that over the holidays.

architeuthidae commented 1 week ago

In summary:

Migen and misoc do need more documentation. I think we already have issues filed there.

This is true, and in Migen there are some issues to this effect, e.g. https://github.com/m-labs/migen/issues/23, as well as the manual’s [To be written] page: https://m-labs.hk/migen/manual/synthesis.html, which is sort of an issue by implication. I can't find an equivalent issue for MiSoC currently, but regardless I suspect that'd be a separate issue for that repository.

Artiq could profit from documentation in a few areas. E.g. writing an rtio driver. I think there is an issue about that as well.

This is done.

It would also be useful to have documentation covering the architecture, both in overview and medium level detail, of how all the various processes are spawned, communicate, etc within ARTIQ -- workers, master, applets, compiler, etc.

This is also done -- a lot of this kind of stuff has now been I hope clarified in the manual (and is even visually represented in the Overview diagram.) With the most recent PR the medium-level detail should now also be covered.

If there's no objection I think this can be closed. Or if there's more specific wants, more specific issues can be opened.