vislearn / FrEIA-community

Community-driven effort to improve and extend FrEIA
0 stars 0 forks source link

Tutorial & documentation #5

Open psteinb opened 3 years ago

psteinb commented 3 years ago

What?

The auto-generated docs for the API and modules are old, and not regularly refreshed. It's also very hard to navigate, and all the inherited torch.nn.Module methods are also included. The tutorial is just a long README, but would be better in separate files on different topics.

How?

Difficulty? 2/5

wapu commented 3 years ago

If we're thinking about restructuring some the modules and the way Jacobian logdets are passed along the structure, the documentation should probably wait for that decision to be made.

tbung commented 3 years ago

I'd suggest we decide on a docstring format that sphinx understands relatively early, so we can start writing docstrings as we make changes (see also the discussion in https://github.com/VLL-HD/FrEIA/pull/54).

As far as I can tell the most common formats are:

The last two are also contrasted on the sphinx extension page. Pytorch uses the Google format.

psteinb commented 3 years ago

needless to say, @tbung brings up a good point. The docstring format should be chosen in light of the HTML rendering engine that will display the tutorial and potentially a API reference.

if no decision between can be reached, you can always go with vanilla jekyll offered by github pages. How code is rendered by this engine, is something beyond my knowledge.

ardizzone commented 3 years ago

To me, the Google docstring format is most legible. https://google.github.io/styleguide/pyguide.html#Comments I would just go ahead and start using it (I promise I will go through and change everything if we decide otherwise)

And for both sphinx and mkdocs there are ways of parsing them (https://github.com/mkdocstrings/mkdocstrings)

wapu commented 3 years ago

I also like the Google format. Any preference between sphinx and mkdocs? Seems like mkdocs is easier to use and sphinx has more features, but both claim to have plugins which replicate the advantages of the other...

psorrenson commented 3 years ago

I am beginning work on fixing up the tutorial. Here is my rough plan:

Anything else or something I should do differently?

ardizzone commented 3 years ago

That sounds perfect! I think it's definitely safe to split it up into multiple .rst or .md files (e.g. one on writing your own InvertibleModule, etc.), so it becomes easier to navigate once it's online.

Also perhaps a small chapter on the basic concept of invertible computation graphs, and that the Reversible* graph wraps such a graph in a torch.nn.Module, and you can access the condition, input, output nodes by calling .forward()? (I don't know if that would be useful or not)

wapu commented 3 years ago

We should definitely provide a clean code example with the full training loop, both unsupervised and conditional, with explanation where adaptations to custom data sets/problems need to be made. But that can be added in the end.

tbung commented 3 years ago

I decided on Sphinx because it seems more stable and battle-tested, also Sphinx allows interdoc links, where we can define some external documentations we want to reference (for example PyTorch) and then easily link to entries in those docs. I also found more projects using Sphinx to draw inspiration from.

We can still use markdown instead of RST, if you prefer.

tbung commented 3 years ago

Do we want a github action to generate and publish docs? If yes, do we also want that to run on every change or do we want to do some kind of release cycle and only update docs on release?

In my opinion automatic docs on releases are the best idea, as long as the project is relatively small and we actually do releases.

ardizzone commented 3 years ago

Sounds good! I think we were planning to do a release once we're done with everything (that would be "0.3" I guess)

tbung commented 3 years ago

I have finished a basic setup (see https://github.com/VLL-HD/FrEIA/pull/68). Points up for discussion are:

ardizzone commented 3 years ago

:star_struck: ooh, that looks very very nice! One thing I noticed: For the modules, the __init__() and the class itself both have a docstring (the former explaining the constructor args, the latter explaining what the module does, giving paper references and so son) Right now, in the docs, only the class docstring is given. Is it better if I put the constructer arguments under the class docstring? Or is there a way for the docs to list both?

It's fine if we collect opinions on the theme, structure, how to serve, but I would give you the mandate to just decide it in the end if no-one has strong thoughts on it.

tbung commented 3 years ago

I have included __init__ functions now, might take a moment to update on Github pages.

Yes, I think I picked sane defaults for our situation, so if nobody has any opinion on those points we can go ahead and merge the PR in a bit.

psorrenson commented 3 years ago

Update on tutorial: