Code is not readable

[interfect]

I went to a random file and decided to look at some random lines:

https://github.com/golosio/annabell/blob/master/src/fssm.h#L51-L57

They define some available methods on the fssm type:

  int ActivWnn();
  int WTA();
  int NewWnn();
  int NewWTA();
  int NumWnn();
  int RndWnn();
  int NextWnn();

Nowhere is it explained what any of these operations are, why they would ever be called, what they are supposed to return, or even what an fssm is supposed to be (besides a kind of ssm). I realize that the meanings of the acronyms would no doubt be clearer to someone with more of a neural networks and NLP background than myself, but even if I knew what things were being referred to and had an idea of what one might want to do with those things, it would still be very challenging to understand how those things are being represented in code.

Also take, for example, these other lines:

https://github.com/golosio/annabell/blob/master/src/fssm.cc#L457-L471

They look like this:

  for (unsigned int iv=0; iv<SparseRefColumn.size(); iv++) {
    for (unsigned int i=0; i<SparseRefColumn[iv]->size(); i++) {
      int iy = SparseRefColumn[iv]->at(i);
      if (iy>=0 && iy<Ny()) {
    for (int ix=0; ix<Nx(); ix++) {
      int inr = Nx()*iy + ix;
      //ActivVect.push_back(inr);
      std::vector<int>::iterator ia=ActivVect.begin();
      while (ia<ActivVect.end() && *ia<=inr) ia++;
      if (ia==ActivVect.begin() || *(ia-1)!=inr) ActivVect.insert(ia,inr);
      Nr[inr]->Activ();
      Nr[inr]->A += GB;
    }
      }
    }

They're clearly doing some kind of 2d loop over something, and calling other Activ() methods from this Activ() method. I would guess that they're passing on activation from a neuron, but if that's correct it's due to what I already know, not what I gleaned from the code, and if it's wrong it's because the code isn't sufficiently clear to disabuse me of my preconceived notions.

If I were checking this code to make sure that it didn't have bugs during the peer review process, I would remain thoroughly unconvinced. If I don't know what it's for, I can't know if it works correctly.

Documentation should be added to every file (to let the reader know what is defined there), to every class (to document what it is and why it is needed), to every method and member (to document what the method or member means and when it is to be used, and what the arguments and return types, if any, are supposed to be), and to every nontrivial code structure (to explain why the code as written would be expected to function correctly, given what the method it's in is specified as doing). Without this documentation, nobody will ever be able to know why (or even whether) the code presented works correctly.

[golosio]

thank you for your interest in this program. Up to a few days ago it was only used for research purpose by a few researcher of my same team, and I did not care too much about (important) implementation issues, including comments. But now that many other researcher are interested in this work, for sure I'll do it as soon as I can. Concerning peer review, we had an article subject to peer review; the reviewer did not have to judge the software, they had to judge the model, and I can tell you that they did it very accurately. If I submitted a paper about the software I would have taken care before about implementation issues, as I did with other projects.
Some quick answers: fssm is just a fast implementation of ssm, which uses some tricks to take advantage of the fact that the signal is sparse, i.e. only a small fractions of the neurons are active at a given time. The lines that you copied above are one of such tricks. The program could work with ssm only, and it would be absolutely equivalent, but it would be much slower. WTA is the winner-take-all rule. I will put comments as soon as I can.
Best, Bruno

[jotadepicas]

Hi @golosio,
I have to second what @interfect was pointint out. It would be of great help if your code was more self-explaning and documented, specially for people that come from the programming world and with a modest background in AI theory. (btw, what does SSM stand for?). In that way, it will be easier for the community to contribute. Congratulations on your project and thanks!

EDIT: I'm reading your paper "A cognitive neural architecture able to learn and communicate through natural language" and found out that SSM stands for "sparse-signal map", right?

[golosio]

Hi jotadepicas,
As I said I will write comments as soon as I have time. SSM stands for sparse-signal map. It is simply a ANN layer (one-dimensional or two-dimensional) with only a small fraction of the neurons active at a time. The signal propagates in the same way as in ordinary ANNs, however the computation can be done more quickly by keeping track of the active neurons.
GOOD THAT YOU ARE READING THE PAPER! I was just going to suggest that. I appreciate the help from people of the programming community, as you, but you have to know what you are doing, also because otherwise, after doing a lot of work, you can realize later that this model is not what you expected it to be.
Indeed, I have to make a clarification, because I know that journalists have written a lot about our work, but they often seek for sensationalism, and what is actually written in our scientific work can be quite different from what journalists wrote about it. Our work is on a cognitive model, not on a dialog system. The purpose is not to obtain the best performance on dialog, but to validate a neural model of how we learn language and how verbal information is processed in our brain. This means that we are not simply simulating the way how humans hold a dialog, we are trying to simulate neural processes that support language development. It can be very interesting if you want to understand how our cognitive functions, as reasoning an language, arises from neural processes. But you should not think that you can use it as a dialog system for entertainment.

[jotadepicas]

Yes, I know. Almost all the articles you refer to are about skynet, the machines taking over and the end of the world :P However, my area of interest is the same as yours. Btw, I've finished all your docs! Is there any forum o something like that to discuss about them? (here?).

[golosio]

Sorry, at the moment I do not use a forum. You can write here or to my email address golosio@uniss.it

[jotadepicas]

Besides documentation, one other thing it would be great to do is to make the code adhere to some coding conventions. I like Google's (https://google.github.io/styleguide/cppguide.html#Type_Names), which is more or less general and covers a lot of topics.

I don't think one should choose to follow a coding convention in a dogmatic way, but whatever convention you choose, should be consistent across all the project. For example, all classes names starting with a capital letter, all variables starting with lowercase, using camel-case everywhere, etc. Makes it easier to navigate the code.

I've just forked the repo, and I'll be happy to contribute in this matter whenever I manage to find the time if you are ok with it :)

[golosio]

Sure. I had a look at Google's, it seems that the main difference between Google's coding convention and the one that I use is that I use lowercase with underscores for temporary variables, and CamelCase for class members (both for variables and functions). Maybe we can agree on a convention that requires less changes to the code and when we have time check that it is applied consistently...

[jotadepicas]

Yes, I'd focus on the big things first. For example, classes should be capitalized and variables viceversa. One example is ssm class should be called SSM and variables and instance members of that type should be ssm.

[jotadepicas]

Hi guys,
I've just made pull request #13 and added doxygen configuration to the project. From there we can start to document the code and generante html docs, uml diagrams and other stuff like that.

[gramcracker]

this looks really great. I'm really into neural networks and artificial learning of semantics . I've even begun my own neural network library, though it needs some work. Id love to learn how this program works, and I also think it would benefit from some documentation .

[golosio]

Thank you Josh,
we are working on writing more comments in the code and making it more readable.
Since you are a programmer, I suggest that you have a look at one of my answers to Issue 1 before devoting time to this project:
#4 (comment)
That said, currently the main documentation are the paper (including the supporting information appendixes at the end):
http://journals.plos.org/plosone/article?id=10.1371/journal.pone.0140866
and the software manual.
Concerning the code, also consider that the most important file of the project is Annabell.cc, which specifies the architecture. If you look in this file you will recognize the architecture represented in the figures of the document "The ANNABELL system architecture" (Appendix 5 of the paper).
Best, Bruno