Skip to content

Conversation

@MSenden
Copy link

@MSenden MSenden commented Feb 7, 2018

AUTHOR
Mario Senden; Jannis Schuecker; Jan Hahne; Markus Diesmann; Rainer Goebel

Dear @ReScience/editors,

I request a review for the following replication:

Original article

Title: A Neural Model of the Saccade Generator in the Reticular Formation.
Author(s): Gancarz, G. & Grossberg, S
Journal (or Conference): Neural Networks
Year: 1998
DOI: 10.1016/S0893-6080(98)00096-3
PDF: Gancarz-1998.pdf

Replication

Author(s): Senden, M., Schuecker, J., Hahne, J., Diesmann, M. and Goebel, R.
Repository: Senden-2018
PDF: Senden-2018
Keywords: saccade generation; rate neurons; reticular formation
Language: Python; NEST
Domain: Neuroscience

Results

  • Article has been fully replicated
  • Article has been partially replicated
  • Article has not been replicated

Potential reviewers


EDITOR

  • Editor acknowledgment (@otizonaizit) 13 February 2018
  • Reviewer 1 (@apdavison) 13 February 2018
  • Reviewer 2 (@gdetor) 16 February 2018
  • Review 1 decision [accept] 23 April 2018
  • Review 2 decision [accept] 23 April 2018
  • Editor decision [accept] 4 May 2018

@MSenden MSenden changed the title Senden Schuecker Hahne Diesmann Goebel 2018 Review Request: Senden Schuecker Hahne Diesmann Goebel 2018 Feb 7, 2018
@MSenden MSenden changed the title Review Request: Senden Schuecker Hahne Diesmann Goebel 2018 Review Request: Senden Schuecker Hahne Diesmann Goebel Feb 7, 2018
@rougier
Copy link
Member

rougier commented Feb 9, 2018

Thanks for your submission. An editor will be assigned soon.

@rougier
Copy link
Member

rougier commented Feb 9, 2018

@otizonaizit Can you edit this submission ?

1 similar comment
@rougier
Copy link
Member

rougier commented Feb 12, 2018

@otizonaizit Can you edit this submission ?

@otizonaizit
Copy link
Member

sure!

@otizonaizit
Copy link
Member

@heplesser : could you review this submission?

@otizonaizit
Copy link
Member

@apdavison : could you review this submission?

@apdavison
Copy link

Possible conflict of interest: I'm a co-PI with Markus Diesmann on a grant (Human Brain Project)

@otizonaizit
Copy link
Member

otizonaizit commented Feb 13, 2018

Possible conflict of interest: I'm a co-PI with Markus Diesmann on a grant (Human Brain Project)

@apdavison : if you were not involved in the work leading to the present submission this is fine, in my opinion, but let's see what @rougier thinks about it...

@otizonaizit
Copy link
Member

by the way, @heplesser may be in a similar conflict, but again, if @rougier agrees with me, I see no conflict of interest if @heplesser was not involved in the work leading to the present submission

@MSenden
Copy link
Author

MSenden commented Feb 21, 2018

@otizonaizit
Thank you for letting the review continue. We have been following the discussion ensuing our submission with interest.

Following your recommendations, we have added the full code of nest v2.16.0-beta to the submission (within the new folder named nest). In this way the reviewers have a chance to examine the implementation more thoroughly as the implementation of every neuron model can be found in ../nest/nest-simulator-master/models.
A new section in the article (appendix A1) is devoted to give a brief overview of how rate-based neurons are implemented in NEST and refers to relevant files which reviewers might be interested in.
We did not add the code as an externally implemented module, however, as this would either be identical to the internal implementation and hence be redundant or differ from the internal implementation and hence not be fully representative of our work.

We also extended the introduction to provide more thorough background as to why we opted to use NEST. With regard to the point of why we consider recently added features tested well enough for readers/reviewers to trust them, we consider the internal review process of the NEST initiative very reliable. More important to the present discussion, however, we would argue that it is precisely through (numerous) successful re-implementations of existing models that the community can get an independent indication as to whether a software tool is reliable.

@otizonaizit
Copy link
Member

@MSenden : thank you for the changes! They look good and will help a lot in the review process!
@apdavison, @gdetor: you can start reviewing the paper (and the code :)))

@rougier
Copy link
Member

rougier commented Mar 13, 2018

@otizonaizit Gentle reminder.

@otizonaizit
Copy link
Member

@apdavison , @gdetor : do you have a timeline for your review? :)

@gdetor
Copy link

gdetor commented Mar 20, 2018

@otizonaizit I'm sorry for the delay. I'll try to finish the review by next week.

@otizonaizit
Copy link
Member

Hi @apdavison , are you still on this? Do you have an approximate timeline for the review? Thanks!

@apdavison
Copy link

@otizonaizit yes, sorry for not replying sooner. I have installed everything and run the simulations; everything seems fine, so I now need to review the code. Unfortunately I have a major deadline for a grant in one week, so I won't be able to finish the review until the first week in April.

@gdetor
Copy link

gdetor commented Apr 3, 2018

The authors reliably reproduced the results of [A Neural Model of the Saccade Generator in the Reticular Formation]. The source code runs smoothly and with no problems and the text is well-written.
The provided instructions for installing NEST on Linux are correct and overall I did not face any issue with their code or the simulations execution.

General comments

  1. How the authors estimated the values from the figures of the original paper in order to compare against their own results?
    Did they use any software (like plot digitizer, http://plotdigitizer.sourceforge.net/) to extract the data points from the figures?
    I think the authors should provide some more information in the text on how they compared their results against the original ones.

  2. The authors in order to integrate the tonic neurons ODE used the Euler-Maruyama method which is mainly used to integrate SDEs. Does this imply that they use some sort of noise in their implementation?

  3. Regarding the discrepancy: What does it mean that it might stem from the input description (pg. 10)? In addition is not clear if F is indeed a function of time or some sort of constant.

Text

  1. The text is well-written and makes clear all the details of the implementation. There are, however, some typos (see below).
  2. Please make clear that the subscripts l, r, u, d mean left, right, up and down.

Typos:

  1. Introduction, pg 1, first sentence -> Commission

  2. Introduction, pg 1, first sentence -> pan-European

  3. Introduction, pg 1, line 6 -> collaboration

  4. Methods, pg 2, line 7 -> Colliculus

  5. Methods, pg 2, equation 2, there is extra parenthesis at the RHS third term.

  6. Results, pg 7, line 4 -> amplitude

  7. Figure 7, pg 8 -> velocity
    The authors might need to proofread the manuscript once again.

  8. In the Methods section, please add the proper punctuation in the equations. That would be nice if the authors write the ODEs in a more elegant form.

Figures

  1. The x,y-labels as well as the ticks labels fonts are too small. Please make it larger.
  2. In addition, rotate y-axis ticks (abbreviations like Input, LLBN, etc) to match the original figures (easier to read).
    Authors can use something like ax.set_ylabel('Input', rotation='horizontal', labelpad=20)
  3. In the caption of Figure 4, did the authors mean staircase continuum instead of continued?
  4. In Figure 5, what is 0.02 and 0.1? Please provide more details regarding this figure.
  5. Figure 6, comments regarding the B panel are missing.
  6. In Figure 7, what do the dashed and the solid lines indicate?

Code

  1. The code provided in the setup_model.py script is well-commented.

  2. The code in all the Exp?.py scripts need some more comments. For instance, t_relax variable, F, W, J.

  3. Time vector T may confuse the reader (it's not immediately clear what is the relaxation time and what is the stimulation (simulation) one).

  4. The authors should provide platform information in README file.
    See http://rescience.github.io/platform/

  5. None of the scripts comply with the PEP8. Authors can use the autoflake tool to make their files comply with PEP8.

  6. The code runs with Python 3 as well. The auhors need to add maybe the following lines in their
    Exp?.py scripts:
    import sys
    if sys.version_info[0] < 3:
    execfile('setup_model.py')
    else:
    exec(open('setup_model.py').read())

  7. In Exp?.py scripts at line 45: (typos) is does -> does

@apdavison
Copy link

I am satisfied that this submission is a full replication of the Gancarz and Grossberg technical report.

Installing NEST was straightforward. I adapted the Python scripts very slightly so they would run with Python 3 (replaced execfile('setup_model.py') with from setup_model import *). All scripts ran quickly and without problems, and produced visually identical figures to those in the article (there were some minor differences in the .eps files, presumably due to using a different version of matplotlib).

I skimmed through the C++ code that was added to NEST for this model, and studied two of the model implementations more closely, without finding any problems.

I think the code should be adapted to run with both Python 2.7 and 3.6 (Python 2.7 will reach end-of-life in about 20 months from now). As both I and @gdetor found, this is trivial to do using either import or exec. In general, I dislike the use of execfile/exec for code readability reasons: it makes it harder for the reader to understand where variables are defined, or to find their definitions, and it prevents IDEs or code editors from helping the user with this. Of course, from X import * is little better; the best approach would be to explicitly import into each script the required variables and functions from setup_model.py.

@otizonaizit
Copy link
Member

@apdavison @gdetor : thank you for your reviews!
@MSenden : could you address the reviewers' comments?

@MSenden
Copy link
Author

MSenden commented Apr 16, 2018

We would like to thank @apdavison @gdetor for taking the time to review our manuscript and their helpful comments. In what follows we provide a point by point response to the issues raised by the reviewers.

Reviewer 1

General comments

How the authors estimated the values from the figures of the original paper in order to compare against their own results? Did they use any software (like plot digitizer, http://plotdigitizer.sourceforge.net/) to extract the data points from the figures? I think the authors should provide some more information in the text on how they compared their results against the original ones.

We are sorry for this shortcoming in the description of our methods. We are familiar with the plot digitizer and the original version of the manuscript indeed uses the tool to extract individual data points from figures 6, 9 and 10 of the original publication. The revised version of the manuscript now mentions this explicitly and describes in more detail the exact information we obtain from the three figures.

The authors in order to integrate the tonic neurons ODE used the Euler-Maruyama method which is mainly used to integrate SDEs. Does this imply that they use some sort of noise in their implementation?

The reviewer is correct to point out that the Euler-Maruyama (EM) method is used to integrate stochastic differential equations. NEST is generally capable of handling noise (using the EE method by default and the EM method in the absence of passive decay). However, there is neither noise in the original model nor in our implementation of the saccade generator and hence the EM method reduces to the forward Euler method. The revised version of the manuscript now states that tonic neurons are integrated using the forward Euler method (rather than the EM method).

Regarding the discrepancy: What does it mean that it might stem from the input description (pg. 10)?

The most important implication of the discrepancy originating from the input description is that there are no issues with the model itself. Once the correct input is provided (the one matching the shape displayed in figure 10 of the original publication), the model produces the expected output. Furthermore, the fact that the discrepancy remains when the input is treated analytically implies that the deviation is not due to our implementation. In the revised version of the manuscript we have improved the corresponding section of the text accordingly.

It is not clear if F is indeed a function of time or some sort of constant

F is a constant but we agree that the equations in the original version of the manuscript suggest that F is a function of time. In the revised version of the manuscript we have changed F(t) to simply read F.

Text

The text is well-written and makes clear all the details of the implementation. There are, however, some typos (see below). Please make clear that the subscripts l, r, u, d mean left, right, up and down.

Thank you for pointing out this omission. The revised manuscript now explains the subscripts. Furthermore, all typos found by the reviewer have been corrected and the manuscript has been thoroughly re-read to check for further typos.

In the Methods section, please add the proper punctuation in the equations.

For the revised version of the manuscript we have rechecked all equations.

That would be nice if the authors write the ODEs in a more elegant form.

The original version of the manuscript uses \frac{d}{dt} to indicate temporal derivatives in line with the style of the work we refer to, the revised manuscript uses \dot{\boxempty} to reduce visual clutter.

Figures

The x,y-labels as well as the ticks labels fonts are too small. Please make it larger

We have adjusted the figures accordingly.

Rotate y-axis ticks (abbreviations like Input, LLBN, etc) to match the original figures (easier to read).

Thank you for the advice, we have done that.

In Figure 5, what is 0.02 and 0.1? Please provide more details regarding this figure.

Sorry. These values are arbitrary and have been removed from the figure

Figure 6, comments regarding the B panel are missing.

This is a misunderstanding, the respective sentence in the original version of the manuscript is not specifying the panels but just stating an observation. In the revised version we have improved the description of the panels and moved the respective sentence to the main text.

In Figure 7, what do the dashed and the solid lines indicate?

The reviewer is right that this is not obvious in the original version of the manuscript. The line styles differentiate between low and high velocity saccades. The revised manuscript makes this explicit.

Code

The code in all the Exp?.py scripts need some more comments. For instance, t_relax variable, F, W, J.

We have added further comments.

Time vector T may confuse the reader (it's not immediately clear what is the relaxation time and what is the stimulation (simulation) one).

The newly added comments explain this better.

The authors should provide platform information in README file. See http://rescience.github.io/platform/

Thank you for pointing this out. We have added this information.

None of the scripts comply with the PEP8. Authors can use the autoflake tool to make their files comply with PEP8.

The scripts now comply with PEP8.

The code runs with Python 3 as well. The auhors need to add maybe the following lines in their Exp?.py scripts: import sys if sys.version_info[0] < 3: execfile('setup_model.py') else: exec(open('setup_model.py').read())

Both reviewers are correct to point out that the code runs with Python 3. This is now explicitly mentioned in the manuscript. In compliance with the second reviewer, we have opted for using from setup_model import * to set up the model in each script.

In Exp?.py scripts at line 45: (typos) is does -> does

We corrected this typo.

Reviewer 2

I think the code should be adapted to run with both Python 2.7 and 3.6 (Python 2.7 will reach end-of-life in about 20 months from now). As both I and @gdetor found, this is trivial to do using either import or exec. In general, I dislike the use of execfile/exec for code readability reasons: it makes it harder for the reader to understand where variables are defined, or to find their definitions, and it prevents IDEs or code editors from helping the user with this. Of course, from X import * is little better; the best approach would be to explicitly import into each script the required variables and functions from setup_model.py.

The code indeed runs with Python 3. This is now explicitly mentioned in the manuscript. Thank you for pointing out the option to use from setup_model import * which we now use in our scripts.

@rougier
Copy link
Member

rougier commented Apr 23, 2018

@otizonaizit @gdetor @apdavison Gentle reminder...

@otizonaizit
Copy link
Member

@gdetor @apdavison : my impression is that the authors have addressed all your points in great detail, so I think I just need a confirmation from you that I can accept the submission as is :)

@apdavison
Copy link

@otizonaizit I was planning to re-run the scripts just to double check there have been no regressions, but otherwise I'm happy for you to accept the submission.

@gdetor
Copy link

gdetor commented Apr 23, 2018

@otizonaizit The authors addressed properly all the issues. All the scripts run now smoothly and out-of-the-box.
However, there are two more typos in the text:
1] Methods section, pg. 2, first paragraph, sentence: "Please note that ..., respectively". The subscripts are not in the proper order.
2] Methods section, pg. 3, last paragraph: forward Euler method -> Forward Euler method

I suggest to you to accept the submission.

@MSenden
Copy link
Author

MSenden commented Apr 24, 2018

@gdetor thanks for endorsing the article. We corrected the typos you mention.

@rougier
Copy link
Member

rougier commented May 2, 2018

@otizonaizit Any update?

@ReScience ReScience locked as resolved and limited conversation to collaborators May 4, 2018
@otizonaizit
Copy link
Member

Article is published with DOI
The article will be soon listed on http://rescience.github.io/read/

@otizonaizit otizonaizit closed this May 4, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.