{"action":"created","issue":{"url":"https://api.github.com/repos/golosio/annabell/issues/4","labels_url":"https://api.github.com/repos/golosio/annabell/issues/4/labels{/name}","comments_url":"https://api.github.com/repos/golosio/annabell/issues/4/comments","events_url":"https://api.github.com/repos/golosio/annabell/issues/4/events","html_url":"https://github.com/golosio/annabell/issues/4","id":116965031,"number":4,"title":"Code is not readable","user":{"login":"interfect","id":752054,"avatar_url":"https://avatars.githubusercontent.com/u/752054?v=3","gravatar_id":"","url":"https://api.github.com/users/interfect","html_url":"https://github.com/interfect","followers_url":"https://api.github.com/users/interfect/followers","following_url":"https://api.github.com/users/interfect/following{/other_user}","gists_url":"https://api.github.com/users/interfect/gists{/gist_id}","starred_url":"https://api.github.com/users/interfect/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/interfect/subscriptions","organizations_url":"https://api.github.com/users/interfect/orgs","repos_url":"https://api.github.com/users/interfect/repos","events_url":"https://api.github.com/users/interfect/events{/privacy}","received_events_url":"https://api.github.com/users/interfect/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":11,"created_at":"2015-11-15T01:01:35Z","updated_at":"2016-01-14T08:55:45Z","closed_at":null,"body":"I went to a random file and decided to look at some random lines:\r\n\r\nhttps://github.com/golosio/annabell/blob/master/src/fssm.h#L51-L57\r\n\r\nThey define some available methods on the `fssm` type:\r\n\r\n```\r\n  int ActivWnn();\r\n  int WTA();\r\n  int NewWnn();\r\n  int NewWTA();\r\n  int NumWnn();\r\n  int RndWnn();\r\n  int NextWnn();\r\n```\r\n\r\nNowhere 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.\r\n\r\nAlso take, for example, these other lines:\r\n\r\nhttps://github.com/golosio/annabell/blob/master/src/fssm.cc#L457-L471\r\n\r\nThey look like this:\r\n\r\n```\r\n  for (unsigned int iv=0; iv<SparseRefColumn.size(); iv++) {\r\n    for (unsigned int i=0; i<SparseRefColumn[iv]->size(); i++) {\r\n      int iy = SparseRefColumn[iv]->at(i);\r\n      if (iy>=0 && iy<Ny()) {\r\n\tfor (int ix=0; ix<Nx(); ix++) {\r\n\t  int inr = Nx()*iy + ix;\r\n\t  //ActivVect.push_back(inr);\r\n\t  std::vector<int>::iterator ia=ActivVect.begin();\r\n\t  while (ia<ActivVect.end() && *ia<=inr) ia++;\r\n\t  if (ia==ActivVect.begin() || *(ia-1)!=inr) ActivVect.insert(ia,inr);\r\n\t  Nr[inr]->Activ();\r\n\t  Nr[inr]->A += GB;\r\n\t}\r\n      }\r\n    }\r\n```\r\n\r\nThey'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.\r\n\r\nIf 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.\r\n\r\nDocumentation 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.\r\n"},"comment":{"url":"https://api.github.com/repos/golosio/annabell/issues/comments/171575701","html_url":"https://github.com/golosio/annabell/issues/4#issuecomment-171575701","issue_url":"https://api.github.com/repos/golosio/annabell/issues/4","id":171575701,"user":{"login":"golosio","id":3852089,"avatar_url":"https://avatars.githubusercontent.com/u/3852089?v=3","gravatar_id":"","url":"https://api.github.com/users/golosio","html_url":"https://github.com/golosio","followers_url":"https://api.github.com/users/golosio/followers","following_url":"https://api.github.com/users/golosio/following{/other_user}","gists_url":"https://api.github.com/users/golosio/gists{/gist_id}","starred_url":"https://api.github.com/users/golosio/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/golosio/subscriptions","organizations_url":"https://api.github.com/users/golosio/orgs","repos_url":"https://api.github.com/users/golosio/repos","events_url":"https://api.github.com/users/golosio/events{/privacy}","received_events_url":"https://api.github.com/users/golosio/received_events","type":"User","site_admin":false},"created_at":"2016-01-14T08:55:45Z","updated_at":"2016-01-14T08:55:45Z","body":"Thank you Josh,\r\nwe are working on writing more comments in the code and making it more readable.\r\nSince 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:\r\nhttps://github.com/golosio/annabell/issues/4#issuecomment-163859240\r\nThat said, currently the main documentation are the paper (including the supporting information appendixes at the end):\r\nhttp://journals.plos.org/plosone/article?id=10.1371/journal.pone.0140866\r\nand the software manual.\r\nConcerning 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).\r\nBest, Bruno\r\n"}}