New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Review Request: Shifman #22

Closed
wants to merge 4 commits into
from

Conversation

Projects
None yet
5 participants
@aaronshifman

aaronshifman commented Oct 15, 2016

Dear @ReScience/editors,

I request a review for the reproduction of the following paper:

  • Ionic Current Model of a Hypoglossal Motoneuron, Purvis LK., Butera RJ., (2005) Journal of Neurophysiology, 93, 723--733

I believe the original results have been faithfully reproduced as explained in the accompanying article.

Thank you for your consideration,
Aaron R. Shifman

Repository at https://github.com/aaronshifman/ReScience-submission/tree/SHIFMAN


EDITOR

  • Editor acknowledgment (@otizonaizit) 25 October 2016
  • Reviewer 1 (@damiendr) 28 October 2016
  • Reviewer 2 (@benoit-girard) 14 November 2016
  • Review 1 decision [accept] 11 January 2017
  • Review 2 decision [accept] 19 December 2016
  • Editor decision [accept] 11 January 2017
@rougier

This comment has been minimized.

Show comment
Hide comment
@rougier

rougier Oct 16, 2016

Member

EDITOR-IN-CHIEF

Thank you for your submission. We'll assign an editor very soon.

Member

rougier commented Oct 16, 2016

EDITOR-IN-CHIEF

Thank you for your submission. We'll assign an editor very soon.

@rougier

This comment has been minimized.

Show comment
Hide comment
@rougier

rougier Oct 17, 2016

Member

EDITOR-IN-CHIEF

@otizonaizit Can you handle this submission ?

Member

rougier commented Oct 17, 2016

EDITOR-IN-CHIEF

@otizonaizit Can you handle this submission ?

@aaronshifman

This comment has been minimized.

Show comment
Hide comment
@aaronshifman

aaronshifman Oct 24, 2016

Hello @rougier I was wondering if there has been any development?

Thank you,
Aaron

Hello @rougier I was wondering if there has been any development?

Thank you,
Aaron

@rougier

This comment has been minimized.

Show comment
Hide comment
@rougier

rougier Oct 24, 2016

Member

Yes actually, sorry for the delay. The editor will handle the review by tomorrow and hopefully, he will assign two reviewers tomorrow as well.

Member

rougier commented Oct 24, 2016

Yes actually, sorry for the delay. The editor will handle the review by tomorrow and hopefully, he will assign two reviewers tomorrow as well.

@aaronshifman

This comment has been minimized.

Show comment
Hide comment
@aaronshifman

aaronshifman Oct 24, 2016

Thank you very much for the quick reply.

Aaron

Thank you very much for the quick reply.

Aaron

@otizonaizit

This comment has been minimized.

Show comment
Hide comment
@otizonaizit

otizonaizit Oct 25, 2016

Member

EDITOR
Thanks @aaronshifman for the submission. I am in the process of assigning reviewers right now.

Member

otizonaizit commented Oct 25, 2016

EDITOR
Thanks @aaronshifman for the submission. I am in the process of assigning reviewers right now.

@otizonaizit

This comment has been minimized.

Show comment
Hide comment
@otizonaizit

otizonaizit Oct 25, 2016

Member

EDITOR

@damiendr, @AdamRTomkins: would you be up for reviewing this submission? Thanks!

Member

otizonaizit commented Oct 25, 2016

EDITOR

@damiendr, @AdamRTomkins: would you be up for reviewing this submission? Thanks!

@damiendr

This comment has been minimized.

Show comment
Hide comment
@damiendr

damiendr Oct 28, 2016

@otizonaizit Yes, I can do it!

@otizonaizit Yes, I can do it!

@otizonaizit

This comment has been minimized.

Show comment
Hide comment
Member

otizonaizit commented Oct 28, 2016

@damiendr : thanks!

@rougier rougier changed the title from Review Request to Review Request: Shifman Nov 1, 2016

@rougier

This comment has been minimized.

Show comment
Hide comment
@rougier

rougier Nov 4, 2016

Member

@AdamRTomkins Can you you tell if you can handle the review or not. No problem if you cannot but we need to know your answer before asking another potential reviewer.

Member

rougier commented Nov 4, 2016

@AdamRTomkins Can you you tell if you can handle the review or not. No problem if you cannot but we need to know your answer before asking another potential reviewer.

@otizonaizit

This comment has been minimized.

Show comment
Hide comment
@otizonaizit

otizonaizit Nov 9, 2016

Member

OK, it seems @AdamRTomkins has gone MIA, so I'm going to look for a different second reviewer.

Member

otizonaizit commented Nov 9, 2016

OK, it seems @AdamRTomkins has gone MIA, so I'm going to look for a different second reviewer.

@otizonaizit

This comment has been minimized.

Show comment
Hide comment
@otizonaizit

otizonaizit Nov 9, 2016

Member

@veezbo: would you be up for reviewing this submission? Thanks!

Member

otizonaizit commented Nov 9, 2016

@veezbo: would you be up for reviewing this submission? Thanks!

@otizonaizit

This comment has been minimized.

Show comment
Hide comment
@otizonaizit

otizonaizit Nov 14, 2016

Member

@aaronshifman: we are a bit unlucky with finding reviewer for this submission, sorry about that! It seems @veezbo s also MIA, so I'll try another reviewer.

Member

otizonaizit commented Nov 14, 2016

@aaronshifman: we are a bit unlucky with finding reviewer for this submission, sorry about that! It seems @veezbo s also MIA, so I'll try another reviewer.

@otizonaizit

This comment has been minimized.

Show comment
Hide comment
@otizonaizit

otizonaizit Nov 14, 2016

Member

@benoit-girard : would you be up for reviewing this submission? Thanks!

Member

otizonaizit commented Nov 14, 2016

@benoit-girard : would you be up for reviewing this submission? Thanks!

@benoit-girard

This comment has been minimized.

Show comment
Hide comment
@benoit-girard

benoit-girard Nov 14, 2016

I think I can do it, but I won't be able to have a look at it before the 21st of November...

I think I can do it, but I won't be able to have a look at it before the 21st of November...

@otizonaizit

This comment has been minimized.

Show comment
Hide comment
Member

otizonaizit commented Nov 14, 2016

@benoit-girard: thank you!

@otizonaizit

This comment has been minimized.

Show comment
Hide comment
@otizonaizit

otizonaizit Nov 14, 2016

Member

@benoit-girard , @damiendr: Could you post your reviews by November 30?

Member

otizonaizit commented Nov 14, 2016

@benoit-girard , @damiendr: Could you post your reviews by November 30?

@damiendr

This comment has been minimized.

Show comment
Hide comment
@damiendr

damiendr Nov 14, 2016

@otizonaizit Yes, will do this week.

@otizonaizit Yes, will do this week.

@benoit-girard

This comment has been minimized.

Show comment
Hide comment
@benoit-girard

benoit-girard Nov 16, 2016

@otizonaizit I will do my best to do so.

@otizonaizit I will do my best to do so.

@damiendr

This comment has been minimized.

Show comment
Hide comment
@damiendr

damiendr Nov 20, 2016

General comments

The manuscript is clear and well presented. I would have liked a bit more background in the intro: what is the original study about? And what constitutes a successful replication for a neuron model? Nothing too verbose, just a couple sentences.

The main script ran without issues on my machine (I checked both Python 2.7.2 and 3.5.1), and took about 5 minutes to re-generate all 9 figures. I can confirm that the curves I reproduce match those that were committed by @aaronshifman. I find only slight differences in axis labels; these are probably due to different fonts or my own .matplotlibrc settings. @rougier @otizonaizit Do we need a policy regarding .matplotlibrc? Here it doesn't make a big difference but I can foresee cases where it would.

In the abstract you link to the cellML model ("here"). I think this should be turned into a reference so that the URL is spelled out in the bibliography.

In the methods you mention setting a maximum timestep because of the step changes in stimulation current. I'm a bit confused because I imagine that the solver would react to the discontinuity by reducing the timestep till it hits the minimum or satisfies the error tolerance. I'm not familiar about that particular solver, so maybe I'm wrong here.

In the results section: "All results from the original paper were faithfully reproduced, however for space concerns...". What you mean to say here, if I understand right, is that the omitted figures aren't hiding anything suspicious. I would rephrase that sentence in light of the discrepancies in fig. 2 and fig. 4 (see more below about that).

Actually, how about an annex section with the omitted figures? @rougier @otizonaizit do we have a max. page count?

In fig. 8 the arrows overlap with the text --- maybe tweak the font size.

I believe article.md and article.pdf should be renamed to author-YEAR as per the guidelines.

Code Review

The code is tidy, well commented and to the point --- there is no doubt what each function is doing. Thanks to the use of scipy's ODE routines there is little ancillary code that could introduce bugs.

One thing that isn't ideal is the way model parameters are defined multiple times: in ode_functions.py and in model.py. The scripts for each figure also redefine a number of these parameters, including some (eg. g_n_bar) that always match the default value. Maybe this is because some parameters go in pairs? At any rate it would be nice to reduce the duplication of the common parameters to a minimum.

A minor thing, run.py with no argument raises an error. This could default to run.py all, in case the reviewer didn't read the README.md first ;-)

Results & replication

The spike train and AP waveform (fig. 3) overlap perfectly with the original (fig. 5), with the exception that the current pulse seems to terminate ~100 ms EDIT: ~400 ms earlier in the replication. On that aspect I am very satisfied with the overal replication claims.

Now, as for the discrepancy in figure 2, I'm a bit skeptical that it's an error in the axis labels --- how likely is it that these were added by hand?

In fact, I find further discrepancies when I try to overlap figure 2b (modified to plot in colour for easy comparison):

mismatch

The green and red curves are fine. But the blue curve definitely doesn't match the original and this affects the total I_Ca too. Also, did I_N and I_P get switched around in the legend?

To me this looks like there is a deeper issue here --- either the original authors reported the wrong parameters for that figure, or there is a mistake in the replication, or there is some other discrepancy (eg. numerical methods). You mention that you were in touch with them. Did you discuss this with them?

Is it possible that this discrepancy could explain the slight mismach you report in figure 4?

I agree that given the general agreement on the shape of the action potentials this is unlikely to change the general claim of replication. But I think the disagreement in fig. 2 requires further work: either (a) identifying a new set of parameter that matches the original or (b) discussing the disagreement further in the manuscript, and changing the conclusion to reflect that.

damiendr commented Nov 20, 2016

General comments

The manuscript is clear and well presented. I would have liked a bit more background in the intro: what is the original study about? And what constitutes a successful replication for a neuron model? Nothing too verbose, just a couple sentences.

The main script ran without issues on my machine (I checked both Python 2.7.2 and 3.5.1), and took about 5 minutes to re-generate all 9 figures. I can confirm that the curves I reproduce match those that were committed by @aaronshifman. I find only slight differences in axis labels; these are probably due to different fonts or my own .matplotlibrc settings. @rougier @otizonaizit Do we need a policy regarding .matplotlibrc? Here it doesn't make a big difference but I can foresee cases where it would.

In the abstract you link to the cellML model ("here"). I think this should be turned into a reference so that the URL is spelled out in the bibliography.

In the methods you mention setting a maximum timestep because of the step changes in stimulation current. I'm a bit confused because I imagine that the solver would react to the discontinuity by reducing the timestep till it hits the minimum or satisfies the error tolerance. I'm not familiar about that particular solver, so maybe I'm wrong here.

In the results section: "All results from the original paper were faithfully reproduced, however for space concerns...". What you mean to say here, if I understand right, is that the omitted figures aren't hiding anything suspicious. I would rephrase that sentence in light of the discrepancies in fig. 2 and fig. 4 (see more below about that).

Actually, how about an annex section with the omitted figures? @rougier @otizonaizit do we have a max. page count?

In fig. 8 the arrows overlap with the text --- maybe tweak the font size.

I believe article.md and article.pdf should be renamed to author-YEAR as per the guidelines.

Code Review

The code is tidy, well commented and to the point --- there is no doubt what each function is doing. Thanks to the use of scipy's ODE routines there is little ancillary code that could introduce bugs.

One thing that isn't ideal is the way model parameters are defined multiple times: in ode_functions.py and in model.py. The scripts for each figure also redefine a number of these parameters, including some (eg. g_n_bar) that always match the default value. Maybe this is because some parameters go in pairs? At any rate it would be nice to reduce the duplication of the common parameters to a minimum.

A minor thing, run.py with no argument raises an error. This could default to run.py all, in case the reviewer didn't read the README.md first ;-)

Results & replication

The spike train and AP waveform (fig. 3) overlap perfectly with the original (fig. 5), with the exception that the current pulse seems to terminate ~100 ms EDIT: ~400 ms earlier in the replication. On that aspect I am very satisfied with the overal replication claims.

Now, as for the discrepancy in figure 2, I'm a bit skeptical that it's an error in the axis labels --- how likely is it that these were added by hand?

In fact, I find further discrepancies when I try to overlap figure 2b (modified to plot in colour for easy comparison):

mismatch

The green and red curves are fine. But the blue curve definitely doesn't match the original and this affects the total I_Ca too. Also, did I_N and I_P get switched around in the legend?

To me this looks like there is a deeper issue here --- either the original authors reported the wrong parameters for that figure, or there is a mistake in the replication, or there is some other discrepancy (eg. numerical methods). You mention that you were in touch with them. Did you discuss this with them?

Is it possible that this discrepancy could explain the slight mismach you report in figure 4?

I agree that given the general agreement on the shape of the action potentials this is unlikely to change the general claim of replication. But I think the disagreement in fig. 2 requires further work: either (a) identifying a new set of parameter that matches the original or (b) discussing the disagreement further in the manuscript, and changing the conclusion to reflect that.

@aaronshifman

This comment has been minimized.

Show comment
Hide comment
@aaronshifman

aaronshifman Nov 20, 2016

Thank you @damiendr for the very thorough comments. I will wait for the other reviewer before I make any changes, however I will address your timestep question here.

SciPy's ODE solver (that I used) is a wrapper around LSODA, which is a variable time step stiffness switching (switches between adams and BDF) algorithm. So you are correct that it will adjust its time step to match relative and absolute tolerances that I define. However when the derivatives are very small it will be much more generous in its time step. Since the current pulse is only 1ms, and the neuron starts (at least numerically) at rest, and the pulse starts at 5ms (if I recall correctly), by the time the solver reaches 5ms it may have a time step longer than 1ms and miss the pulse completely. The very small max time step does away with that worry.

I could have started that pulse at 0 and appended 5ms of rest behind it, but this seemed simpler to explain and more like a "real experiment"

Also on an unrelated note: how did you overlap the two figures so well?

Thanks,
Aaron

aaronshifman commented Nov 20, 2016

Thank you @damiendr for the very thorough comments. I will wait for the other reviewer before I make any changes, however I will address your timestep question here.

SciPy's ODE solver (that I used) is a wrapper around LSODA, which is a variable time step stiffness switching (switches between adams and BDF) algorithm. So you are correct that it will adjust its time step to match relative and absolute tolerances that I define. However when the derivatives are very small it will be much more generous in its time step. Since the current pulse is only 1ms, and the neuron starts (at least numerically) at rest, and the pulse starts at 5ms (if I recall correctly), by the time the solver reaches 5ms it may have a time step longer than 1ms and miss the pulse completely. The very small max time step does away with that worry.

I could have started that pulse at 0 and appended 5ms of rest behind it, but this seemed simpler to explain and more like a "real experiment"

Also on an unrelated note: how did you overlap the two figures so well?

Thanks,
Aaron

@damiendr

This comment has been minimized.

Show comment
Hide comment
@damiendr

damiendr Nov 21, 2016

In Photoshop or GIMP. Take screenshots, import as layers, set opacity/blending mode, align the shoulders of the curves, set that point as transform center, scale Y to match axis labels, scale X to match curves.

In Photoshop or GIMP. Take screenshots, import as layers, set opacity/blending mode, align the shoulders of the curves, set that point as transform center, scale Y to match axis labels, scale X to match curves.

@damiendr

This comment has been minimized.

Show comment
Hide comment
@damiendr

damiendr Nov 22, 2016

@aaronshifman thank you for the explanation on the solver and max timestep, now I see the reason why you need a maximum timestep. I think what got me confused is the mention of the discontinuity: this gives the impression that you need a maximum timestep because the current pulse is a square wave. But if I understand right the issue would occur with any short pulse, even a smooth one.

@aaronshifman thank you for the explanation on the solver and max timestep, now I see the reason why you need a maximum timestep. I think what got me confused is the mention of the discontinuity: this gives the impression that you need a maximum timestep because the current pulse is a square wave. But if I understand right the issue would occur with any short pulse, even a smooth one.

@aaronshifman

This comment has been minimized.

Show comment
Hide comment
@aaronshifman

aaronshifman Nov 22, 2016

@damiendr Yes, there is always the risk that an adaptive stepping algorithm could miss a short pulse, a fixed (maximal) time step is the easiest way around it. I see how the wording could be confusing, I will address this in the next manuscript version.

@damiendr Yes, there is always the risk that an adaptive stepping algorithm could miss a short pulse, a fixed (maximal) time step is the easiest way around it. I see how the wording could be confusing, I will address this in the next manuscript version.

@benoit-girard

This comment has been minimized.

Show comment
Hide comment
@benoit-girard

benoit-girard Nov 23, 2016

Manuscript

I also had problem with the reference to cellML : first, we do no see explicitely the URL, and second, I do not understand clearly the sentence: did you provide a non-functionning cellML description? or the authors? or someone else?

1st line of Methods "author's description" do you mean "authors' description"?

Methods, about the initial conditions: you refer to insufficient 2 significant display figures: in the original paper's table 1, I see 3 to 4 such figures.

Code

I could run the code on a mac with OS X 10.10.5, having obsolete versions of matplotlib (1.4.3) numpy (1.9.2) and scipy (0.13.0b1) did not prevent me from generating the figures (the "all" flag did not work, but I could generate them one by one).

The code itself is quite clear and well organized.

Results

There are no space concerns for ReScience, thus I see no reason to exclude some figures from the manuscript.

Concerning fig 1: the peak of the ADP seems to be a bit lower than -65mV, while in the orignal paper, it seems to reach this value. This small discrepancy may explain some of the following ones.

Concerning fig 2 (of the original manuscript and of the replication): there seem to be important differences in panel B, where the minimal value of the I-Ca current seems larger than in the original paper, and some differences also appear for I-P and I-N, which seem to cross in the replication, while I-P is always smaller than I-N in the original. These of course should explain the discrepency in panel D. The cause for these differences should be investigated, and at least be commented in details (in the replication paper, only the panel D difference is highlighted). Given the differences observed in panel B, I am not sure at all than panel D difference is a matter of original label error.

Concerning fig 3 (of the original paper): the ADP for 0.1uS seems to be almost -65mV in the original graph, but lower in the replication. A good reason to include this figure in the man text and to discuss it.

Concerning fig 5 (original)/fig 3 (replication), the last potential plateau (without spike) lasts more than 500ms in the original, less in the replication. Again a discrepancy to be discussed.

In summary, these differences should be investigated, to either identify and solve the problems, or to make them more explicit in the manuscript of the replication.

Manuscript

I also had problem with the reference to cellML : first, we do no see explicitely the URL, and second, I do not understand clearly the sentence: did you provide a non-functionning cellML description? or the authors? or someone else?

1st line of Methods "author's description" do you mean "authors' description"?

Methods, about the initial conditions: you refer to insufficient 2 significant display figures: in the original paper's table 1, I see 3 to 4 such figures.

Code

I could run the code on a mac with OS X 10.10.5, having obsolete versions of matplotlib (1.4.3) numpy (1.9.2) and scipy (0.13.0b1) did not prevent me from generating the figures (the "all" flag did not work, but I could generate them one by one).

The code itself is quite clear and well organized.

Results

There are no space concerns for ReScience, thus I see no reason to exclude some figures from the manuscript.

Concerning fig 1: the peak of the ADP seems to be a bit lower than -65mV, while in the orignal paper, it seems to reach this value. This small discrepancy may explain some of the following ones.

Concerning fig 2 (of the original manuscript and of the replication): there seem to be important differences in panel B, where the minimal value of the I-Ca current seems larger than in the original paper, and some differences also appear for I-P and I-N, which seem to cross in the replication, while I-P is always smaller than I-N in the original. These of course should explain the discrepency in panel D. The cause for these differences should be investigated, and at least be commented in details (in the replication paper, only the panel D difference is highlighted). Given the differences observed in panel B, I am not sure at all than panel D difference is a matter of original label error.

Concerning fig 3 (of the original paper): the ADP for 0.1uS seems to be almost -65mV in the original graph, but lower in the replication. A good reason to include this figure in the man text and to discuss it.

Concerning fig 5 (original)/fig 3 (replication), the last potential plateau (without spike) lasts more than 500ms in the original, less in the replication. Again a discrepancy to be discussed.

In summary, these differences should be investigated, to either identify and solve the problems, or to make them more explicit in the manuscript of the replication.

@aaronshifman

This comment has been minimized.

Show comment
Hide comment
@aaronshifman

aaronshifman Nov 23, 2016

Thank you @damiendr and @benoit-girard for you reviews and helpful comments. I will implement your requested changes and do what I can to address your concerns.

Thank you @damiendr and @benoit-girard for you reviews and helpful comments. I will implement your requested changes and do what I can to address your concerns.

@aaronshifman

This comment has been minimized.

Show comment
Hide comment
@aaronshifman

aaronshifman Dec 4, 2016

Hello @damiendr and @benoit-girard. I would like to thank you again for your comments on the manuscript. I have commented on code and method changes, and have not itemized grammatical changes.

@damiendr

  1. I have amended the introduction with a few more details
  2. I have changed the run file to default to "all" if the wrong (or no) options are provided.
  3. The cellML model has been cited properly
  4. I have included and discussed all figures in the manuscript
  5. I have fixed all figures to display better. As there has no talk of the matplotlibrc, I have left display settings to the individual user
  6. I have renamed the files to match ReScience rules
  7. I have minimized parameter and code duplication by setting defaults
  8. As for the length of the plateau in figure 5, it ends when I turn off the stimulus, I have increases the stimulus length by 400ms

@benoit-girard

  1. I have addressed the cellML reference
  2. As far as the initial conditions go, many only have 1 sig-fig. However there are other issues. I have amended the text to reflect that.
  3. I do not know why the all flag doesn't work on mac... Unfortunately I cannot debug that. However as it is now, if the code can't figure out the parameter it will default to All, which will hopefully address this issue.

As for the issue of the differences in the implementation. The authors provided me with their original XPP code which has results much closer to my implementation than the original manuscript, which leads me to believe that the original manuscript has a misrepresented parameter somewhere. I have addressed this in the text.

Thank you again for your comments,
Aaron

Hello @damiendr and @benoit-girard. I would like to thank you again for your comments on the manuscript. I have commented on code and method changes, and have not itemized grammatical changes.

@damiendr

  1. I have amended the introduction with a few more details
  2. I have changed the run file to default to "all" if the wrong (or no) options are provided.
  3. The cellML model has been cited properly
  4. I have included and discussed all figures in the manuscript
  5. I have fixed all figures to display better. As there has no talk of the matplotlibrc, I have left display settings to the individual user
  6. I have renamed the files to match ReScience rules
  7. I have minimized parameter and code duplication by setting defaults
  8. As for the length of the plateau in figure 5, it ends when I turn off the stimulus, I have increases the stimulus length by 400ms

@benoit-girard

  1. I have addressed the cellML reference
  2. As far as the initial conditions go, many only have 1 sig-fig. However there are other issues. I have amended the text to reflect that.
  3. I do not know why the all flag doesn't work on mac... Unfortunately I cannot debug that. However as it is now, if the code can't figure out the parameter it will default to All, which will hopefully address this issue.

As for the issue of the differences in the implementation. The authors provided me with their original XPP code which has results much closer to my implementation than the original manuscript, which leads me to believe that the original manuscript has a misrepresented parameter somewhere. I have addressed this in the text.

Thank you again for your comments,
Aaron

@rougier

This comment has been minimized.

Show comment
Hide comment
@rougier

rougier Dec 18, 2016

Member

@otizonaizit Do we have any decision on this ?

Member

rougier commented Dec 18, 2016

@otizonaizit Do we have any decision on this ?

@benoit-girard

This comment has been minimized.

Show comment
Hide comment
@benoit-girard

benoit-girard Dec 19, 2016

I am fine with the new version of the paper, the necessary clarifications have been added.
I recommend acceptation as is.

I am fine with the new version of the paper, the necessary clarifications have been added.
I recommend acceptation as is.

@otizonaizit

This comment has been minimized.

Show comment
Hide comment
@otizonaizit

otizonaizit Dec 19, 2016

Member

@damiendr : can you post your recommendation?

Member

otizonaizit commented Dec 19, 2016

@damiendr : can you post your recommendation?

@aaronshifman

This comment has been minimized.

Show comment
Hide comment
@aaronshifman

aaronshifman Jan 4, 2017

@otizonaizit I realize that the holidays have just ended, but is there any update to this?

@otizonaizit I realize that the holidays have just ended, but is there any update to this?

@otizonaizit

This comment has been minimized.

Show comment
Hide comment
@otizonaizit

otizonaizit Jan 5, 2017

Member

@damiendr: could you give an estimate on when will you be able to give us your final recommendation for the manuscript?
@aaronshifman: I am waiting for the final recommendation from one reviewer to make a decision. In the meanwhile could you please fix the conflicts in your branch? That would help a lot for the publishing process ;-)

Member

otizonaizit commented Jan 5, 2017

@damiendr: could you give an estimate on when will you be able to give us your final recommendation for the manuscript?
@aaronshifman: I am waiting for the final recommendation from one reviewer to make a decision. In the meanwhile could you please fix the conflicts in your branch? That would help a lot for the publishing process ;-)

@rougier

This comment has been minimized.

Show comment
Hide comment
@rougier

rougier Jan 5, 2017

Member

@otizonaizit @aaronshifman I think the conflcts come from modification of the submission template. Since we won't merge the PR anyways, it's not a problem I think.

Member

rougier commented Jan 5, 2017

@otizonaizit @aaronshifman I think the conflcts come from modification of the submission template. Since we won't merge the PR anyways, it's not a problem I think.

@damiendr

This comment has been minimized.

Show comment
Hide comment
@damiendr

damiendr Jan 5, 2017

Sorry @aaronshifman and @otizonaizit for the long wait! Here is my update.

The author has addressed my concerns and the article is much improved — it is especially nice to see all the figures in the text.

Just one minor issue:

Given the fact that the paper had the same parameters as this implementation and the XPP simulation matches this implementation, its likely that some simulations had a slightly different parameter set.

In that sentence it's unclear what "some simulations" refers to — I would be more precise and talk about some figures in the original paper.

And one stylistic point:

It is important to note however that this in no way detracts...

The wording sounds a bit defensive to me — and you don't need to be, because the overall results do match the original, as you explain. Better to turn this around to a factual statement and say that your implementation proves that the model is valid despite these discrepancies, since you can reproduce all the major claims (freq adaptation etc) using the equations and parameters reported in the original paper.

With these corrections I recommend that the paper be accepted.

damiendr commented Jan 5, 2017

Sorry @aaronshifman and @otizonaizit for the long wait! Here is my update.

The author has addressed my concerns and the article is much improved — it is especially nice to see all the figures in the text.

Just one minor issue:

Given the fact that the paper had the same parameters as this implementation and the XPP simulation matches this implementation, its likely that some simulations had a slightly different parameter set.

In that sentence it's unclear what "some simulations" refers to — I would be more precise and talk about some figures in the original paper.

And one stylistic point:

It is important to note however that this in no way detracts...

The wording sounds a bit defensive to me — and you don't need to be, because the overall results do match the original, as you explain. Better to turn this around to a factual statement and say that your implementation proves that the model is valid despite these discrepancies, since you can reproduce all the major claims (freq adaptation etc) using the equations and parameters reported in the original paper.

With these corrections I recommend that the paper be accepted.

@aaronshifman

This comment has been minimized.

Show comment
Hide comment
@aaronshifman

aaronshifman Jan 5, 2017

Thank you @damiendr that's simple enough to do.

Thank you @damiendr that's simple enough to do.

@aaronshifman

This comment has been minimized.

Show comment
Hide comment
@aaronshifman

aaronshifman Jan 10, 2017

Hello @damiendr I have completed your last requested revisions.

Hello @damiendr I have completed your last requested revisions.

@damiendr

This comment has been minimized.

Show comment
Hide comment
@damiendr

damiendr Jan 11, 2017

@aaronshifman thank you for the modifications!
@otizonaizit I can now give my final recommendation, which is to accept the paper.

@aaronshifman thank you for the modifications!
@otizonaizit I can now give my final recommendation, which is to accept the paper.

@otizonaizit

This comment has been minimized.

Show comment
Hide comment
@otizonaizit

otizonaizit Jan 11, 2017

Member

EDITOR
@aaronshifman: congratulations, I hereby accept the submission
@damiendr, @benoit-girard: thanks to the reviewers for their helpful comments, I think they helped in improving the paper significantly.

I'll publish the paper in the next days.

Member

otizonaizit commented Jan 11, 2017

EDITOR
@aaronshifman: congratulations, I hereby accept the submission
@damiendr, @benoit-girard: thanks to the reviewers for their helpful comments, I think they helped in improving the paper significantly.

I'll publish the paper in the next days.

@rougier

This comment has been minimized.

Show comment
Hide comment
@rougier

rougier Jan 19, 2017

Member

🛎 Reminder

Member

rougier commented Jan 19, 2017

🛎 Reminder

@otizonaizit

This comment has been minimized.

Show comment
Hide comment
@otizonaizit

otizonaizit Jan 20, 2017

Member

EDITOR
The article has been published! DOI: http://doi.org/10.5281/zenodo.254145
It will appear in short here

Member

otizonaizit commented Jan 20, 2017

EDITOR
The article has been published! DOI: http://doi.org/10.5281/zenodo.254145
It will appear in short here

@pdebuyl pdebuyl referenced this pull request Apr 4, 2017

Closed

page numbers #47

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment