Review Request: Le Masson, Alexandre #21
Conversation
|
Thanks @maekclena I will assign reviewers soon. |
|
The authors reproduce successfully the results of the original paper on 2 of the 4 proposed tasks. The other tasks (category matching and vibrotactile discrimination) would not bring much to prove that the model is correctly reproduced, so this is fine. The model itself is actually quite simple, reproducing the tasks (especially the probabilistic task) seems to have been the main difficulty in the reimplementation. There were no dependency problem to run the simulations, everything is standard and runs well (tested under linux). The code for the model is very well organized, clear and commented. There are a few docstrings missing in the classes, but the names of the methods are self-explaining anyway. article The article is well written and describes the difficulty of the reimplementation. A few words on the implementation itself could have been nice: why classes for the populations and projections instead of plain numpy arrays (what could have been slightly more readable)? Does In the results section, you do not provide the setup for computing the success rates and convergence speed. How many networks were used? What is the variance? You could also describe slightly more in the text in how far Figs. 1 and 2. reproduce the original figures: it is not obvious without reading the original article that fixate dominates until the go signal, after which the correct action is chosen. Don't feel like you have to do it, but it would have been also nice to reproduce other figures, such as 2C or the upper part of 2D and 4C.
It is indeed not clear in the original article whether the feedback weights are also learned (as they seem to imply), or if they just copy the feedforward ones. What was used in the original implementation? So much for the biological plausibility: as the weights are randomly initialized, there is no chance that the feedforward and feedback weights take the same value in this learning setup. So the superiority over backpropagation is not that obvious... Small typos:
code
python3 simulation.py -h...as the scripts are not directly executable.
from __future__ import print_functionat the beginning of # from statistics import median
from numpy import median |
|
Many thanks to @vitay for the review. I will have to locate another 2nd reviewer, so please bear with me @maekclena while I try to track somebody down. |
|
EDIT: Please see below.
|
|
Hi @oliviaguest; I'd be interested to do the review. |
|
Excellent! Go ahead! |
|
You are too fast for me! Next time... |
|
Any updates on this @eroesch? Also have you had any time to address some of the above @maekclena? |
|
No, I haven't had time yet. I will wait for @eroesch's review to make all necessary corrections at once. |
|
I am almost done. |
|
@eroesch 🔔 |
|
Don't pay attention to the conflict, I just updated the submission directory. No incidence at all on this submission. @oliviaguest @eroesch Any chance for an update ? |
|
I am for accepting the paper, the changes answered my questions. |
|
I am accepting the paper, the changes answered (most of) my questions. |
|
Dear @falex33 and @maekclena — Congratulations! How Attention Can Create Synaptic Tags for the Learning of Working Memories in Sequential Tasks has been accepted! Publication and more details will soon follow. |
|
Hi all — I have attempted my first go at this: |
|
Hi @oliviaguest The archived PDF has wrong links for the red buttons "Article repository", etc. on the first page. The first one, for instance, points to https://github.com/ReScience-Archives/Le_Masson-Alexandre-2016/article whereas the "good" link would be https://github.com/ReScience-Archives/Le_Masson-Alexandre-2016/tree/master/article |
|
Sorry about this. No idea how to fix this problem unfortunately as this is the first time I've done this. I won't try anything until somebody who knows more advises. Perhaps @rougier knows more? |
|
The links are in the header of the Sorry @rougier , we'll need an opinion :-) |
|
I think you need to regenerate the right PDF and upload a new version to Zenodo (while keeping the same DOI). |
|
Indeed. This is why I asked for your more experienced opinion. So unless I am mistaken, which is possible... I'm not sure that I can change any files without emailing Zenodo. I am of course more than happy to email them myself to fix this problem!
|
|
Can't you release a new version on Zenodo using the same DOI ? If not, we'll have to issue a new DOI and update the website and ReScience repository. |
|
I'm unfamiliar with Zenodo but I don't think so...? |
|
Just checked and you'r right. We'll have to contact Zenodo. Can you handle it ? |
|
I think before emailing we/I should fix the repo which we can edit ourselves first. So I need to edit the md file to correct the link, and recompile the pdf. Is there anything else that needs updating? Does the release on github allow changes? |
|
No, I think you will have to make a new release (and double check links before making the release). I can help you on checking the PDF. We really need to automate this part to avoid future errors. I did the same for one submission but I was lucky enough to realizd something was wrong before submitting to Zenodo. |
|
OK, I'll try on Monday! |
|
Zenodo lets you upload a new file and declare that it renders an earlier one obsolete. That's probably the best and most transparent way to handle corrections. If I remember correctly, you have to
|
|
I see Is this a kind of "official" replacement method? Will there be two DOIs? |
|
@khinsen could you point me to where that option is, please? |
|
It's under "Related/alternate identifiers". There you can enter URLs or DOIs of related things and assign a role to them. The two relevant roles are "is new version of this upload" and "is previous version of this upload". For an example of how this is presented to the reader, see this software upload. Under "related identifiers" in the right column you can see both "previous versions" and "new versions". @pdebuyl: Each upload has its own DOI. There are just annotated links between the two uploads. It's more of a versioning than a replacement method, but that's how the scientific record should work in my opinion. |
|
OK, I am attempting step one now of @khinsen's solution. Can somebody check that I do the github-side of things correctly please before I do anything on Zenodo? Thanks for the help so far. Will post a link when I have compiled the corrected |
|
OK, so I think this has now fixed the link problem: https://github.com/ReScience-Archives/Le_Masson-Alexandre-2016/tree/master/article |
|
I think a link for the notebook in the PDF while there is no notebook, do you know why ? Was it the case with previous version ? |
|
Better? 😄 |
|
As the article is in re-processing (okay, because of me :-/) is it useful to have the link to |
|
Removed it. Is the repo and article looking done now? Do we need the |
AUTHOR
Dear @ReScience/editors,
I request a review for the replication of the following paper:
I believe the original results have been faithfully replicated as explained in the accompanying article.
Repository lives at https://github.com/maekclena/ReScience-submission/tree/Le_Masson-Alexandre-2016
EDITOR
09 August 2016)12 August 2016)8 September 2016)30 November, 20161 December, 20161 December, 2016