nengo / nengo

A Python library for creating and simulating large-scale brain models
https://www.nengo.ai/nengo
Other
816 stars 175 forks source link

Docstring standard for networks #563

Open Seanny123 opened 9 years ago

Seanny123 commented 9 years ago

I propose imitating @drasmuss docstring standard for defining networks as seen in his HRL code. This is somewhat related to #231

The standard is as follows:

The benefits of this standard are:

If everyone agrees to this standard I will:

example docstring for the integrator network:

    Parameters
    ----------------
   recurrent_tau:int
       The post-synaptic time constant to be used on the recurrent connection
    n_neurons : int
        Number of neurons to be used, in total.
    dimensions : int
        The number of dimensions of the input and output vectors.

   Inputs and Ouputs
   -------------------------
   input : input
      The input to integrate
   ensemble: output
     The integrated value

My questions involve how easy this is to implement with numpydoc and if there's any additional configuration that we need to put into numpydoc to make it look pretty. @tbekolay, I believe you're the authority on this matter.

tbekolay commented 9 years ago

This all sounds good to me! :rainbow: Could you provide an example docstring to make sure I'm understanding the proposal right?

My understanding is that it will look decent with numpydoc as is (it'll get rendered like any other section) but we could add some additional things to make them render a certain way if we want. The rendering is definitely post-release, but having the docstrings standardized by the release would be awesome.

Seanny123 commented 9 years ago

@tbekolay I've added an example. I think mixing the inputs and outputs is a good idea, because certain inputs and outputs may be related and should be put side-by-side. I'm not super-convinced of this. What do you think?

tbekolay commented 9 years ago

Hmm... I kind of like having separate inputs and outputs; if I'm writing a model I'll only care about one or the other at a time, so it's easier to scan to what I want if they're in separate sections. If I'm just reading through to figure out how the network works, I'm probably fine whichever way it's listed. Separating them also frees up the other side to be used for the type of the input / output. Or maybe that other side should instead give the dimensionality? Or both?

Parameters
----------
recurrent_tau : int
    The post-synaptic time constant to be used on the recurrent connection
n_neurons : int
    Number of neurons to be used, in total.
dimensions : int
    The number of dimensions of the input and output vectors.

Inputs
------
input : Node
    The input to integrate

Outputs
-------
ensemble : Ensemble
    The integrated value

or with dimensionality

input : ``dimensions``
    The input to integrate

ensemble : ``dimensions``
    The integrated value

or both

input : Node, size_in=``dimensions``
    The input to integrate

ensemble : Ensemble, size_out=``dimensions``
    The integrated value
Seanny123 commented 9 years ago

+1 for both and keeping them separate. Should we run it by the rest of the group on Friday or should I just hack it together now?

jgosmann commented 9 years ago

Another way of including both types of information and being more consistent with our keyword args would be:

input : Node(size_in=``dimensions``)
    The input to integrate

ensemble : Ensemble(dimensions=``dimensions``)
    The integrated value
Seanny123 commented 9 years ago

@jgosmann I'm usually a fan for consistency, but I'm worried this time using the keyword arg syntax might cause confusion. I like @tbekolay's better because it treats the two attributes as more separate facts, while yours feels like a deceptive summary. That being said, these are very wishy-washy feelings. I'll bring it up in the meeting tomorrow and we can decide it then.

Seanny123 commented 9 years ago

As discussed in the meeting, Node vs. Ensemble isn't very useful if you're considering the use-case where someone just needs to see how to connect to the network, so we've decided to commit to inputs and outputs separated and only noting the dimensionality. I will create the pull request modifying the existing networks and documenting this process later.