[Re] An anatomically constrained neural network model of fear conditioning
davinellulinvega opened this issue · 27 comments
Original article:
Armony, J. L., Servan-Schreiber, D., Cohen, J. D., & LeDoux, J. E. (1995). An anatomically constrained neural network model of fear conditioning. Behavioral Neuroscience, 109(2), 246–257. https://doi.org/10.1037/0735-7044.109.2.246
PDF URL:
https://github.com/davinellulinvega/ReArmony/blob/master/article.pdf
Metadata URL:
https://github.com/davinellulinvega/ReArmony/blob/master/metadata.yaml
Code URL:
https://gitlab.com/davinellulinvega/armony
Scientific domain:
Computational Neuroscience
Programming language:
Python
Suggested editor:
@eroesch
Thanks for your submission. An editor will be soon assigned.
@eroesch @oliviaguest @benoit-girard Can one of you edit this submission ?
In fact, I have done some work using the original paper myself at some point :)
Well thank you very much in advance for you time and consideration.
@eroesch : I may serve as a reviewer, let me know if you want me to review this one.
Thanks @benoit-girard (@rougier). Actually, I hadn't realised when I accepted the editing of that one, that I have actually interacted with the authors a while back, as a examiner for a phd, on a slightly different version of the code. I had then suggested they submit to ReSci C. I am happy to carry on as editor (and I am almost done scrutinising the code, which is straightforward), but perhaps it would be sensible to have a second pair of eyes, if you have time to spare? @rougier, do you have an opinion?
For me, I think it is fine. Especially since our review are open anyway. If you favor the author, everyone will know :)
Just realised this is two years old and no message since then. Very sorry about such huge delay.
@davinellulinvega Are you still interested in having you work reviewed?
@rougier I am indeed still interested in having this article published, if possible.
@davinellulinvega We'll do your best and again deepest apologies. @eroesch can you go on with editing? @benoit-girard proposed himself as reviewer and I can be the second one.
And I'm afraid we alreaky broke the record of longest submission time...
I can deliver a first review of this work.
The paper, it is clear and well organized.
Concerning the twofold increase of the amygdala activity reported in the original paper, it is hard to judge if it is indeed the case in the replication, based on fig. 7 only. Could the authors replicate fig. 4A of the original paper?
In the paper, the authors refer to themselves with the first person ("I", "me") while there are two of them. Wouldn't it be more coherent to either write "we" or indicate which author is writing what in the paper? It would be unethical to publish a paper with two authors if only one of them performed the study.
Code
The install command for requirements "> pip install -U -r requirements.txt" returns an error : "install: illegal option -- U" could this line be corrected?
Hi! Yes, I can take over the editing.
@benoit-girard Thank you very much for your feedback. I have updated the article's content to make Figure 7 clearer, and removed any mention of "I".
Regarding the repository, I have updated the Readme as well to remove the -U when setting up the requirements with pip.
@davinellulinvega thanks, I now have an error suggesting that the install of pytables is not working:
`girard$ python3 main.py
INFO:root:Executing the development phase ...
INFO:root:Done
INFO:root:Frequency chosen to be the CS: 14
INFO:root:Executing the conditioning phase ...
INFO:root:Done
INFO:root:Recording the results ...
Traceback (most recent call last):
File "/usr/local/lib/python3.11/site-packages/pandas/compat/_optional.py", line 142, in import_optional_dependency
module = importlib.import_module(name)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/local/Cellar/python@3.11/3.11.3/Frameworks/Python.framework/Versions/3.11/lib/python3.11/importlib/init.py", line 126, in import_module
return _bootstrap._gcd_import(name[level:], package, level)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "", line 1206, in _gcd_import
File "", line 1178, in _find_and_load
File "", line 1142, in _find_and_load_unlocked
ModuleNotFoundError: No module named 'tables'
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "/Users/girard/Evaluations/ReScience-Armony-2024/armony-master/main.py", line 206, in
extract_and_save()
File "/Users/girard/Evaluations/ReScience-Armony-2024/armony-master/main.py", line 167, in extract_and_save
final.to_hdf(DATA_PATH, key=k, complevel=9, mode='a',
File "/usr/local/lib/python3.11/site-packages/pandas/core/generic.py", line 2682, in to_hdf
pytables.to_hdf(
File "/usr/local/lib/python3.11/site-packages/pandas/io/pytables.py", line 302, in to_hdf
with HDFStore(
^^^^^^^^^
File "/usr/local/lib/python3.11/site-packages/pandas/io/pytables.py", line 560, in init
tables = import_optional_dependency("tables")
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/local/lib/python3.11/site-packages/pandas/compat/_optional.py", line 145, in import_optional_dependency
raise ImportError(msg)
ImportError: Missing optional dependency 'pytables'. Use pip or conda to install pytables.`
However I installed HDF5 and the installation of the requirements with pip install -r requirements.txt resulted in the installation of:
Successfully installed blosc2-2.4.0 msgpack-1.0.7 ndindex-1.7 numexpr-2.8.8 py-cpuinfo-9.0.0 tables-3.9.2
Maybe pytables was not installed? Is it correctly checked for in the requirements?
@benoit-girard That is a very odd error. Pytables is explicitly stated in the requirements.txt (under the simple name of tables). Plus, it seems that your execution of pip install -r requirements.txt has successfully setup tables.
What kind of environment are you using to test the script? I just made a new pyenv virtual environment on linux to try and reproduce the error, but everything seems to be running fine.
I was trying to have the code running with a macOS system. I switched to Linux and it worked smoothly. The review of the code should arrive soon.
The code is very clear and commented, I don't think any modification is necessary.
I could run the code with a Ubuntu 22.04.1 system, following the installation steps. I could then generate figures identical to those in the paper.
This closes my review.
@benoit-girard Thank you very much for your review.
To try and avoid any issues on different OSs, I have added the hdf5 dependency on the pandas package in the requirements file.
Installation
I'm on OSX and beside the virtualenv step (that I skipped), everything went smoothly. I had to adapt the apt-get with brew and only had to install hdf5 instead of hdf5-tools.
Running
No problem and I check all the figures are identical to the ones in the article.
Code
Code is exceptionally clear (and commented). Well done. You need to add a License to your code though (else it is cannot be read). Also, foir publication, you'll need to have your repository saved on Software Heritage
Article
Article is well written and very easy to follow. Authors explain both the context, the goal and the results. They underlined missing information from the original and the values they have been using (which is one of the goal of the replication) and confirm results on the original paper.
Some questions
- Figure 1 is your own, right (else we need ot ask permission from original pulisher). You could indicate size of structures (with n = xxx) but not mandatory at all. Figure is nice as it is.
- You explain that size of layers were not given but aren't they given by figure 1 in the original article ?
Conclusion
@eroesch Article can be published provided a license is added to the code.
@davinellulinvega It's a shame it took us more than two years to make the review. Again, very sorry for that.
@davinellulinvega Really sorry for the delay, it's really my fault!
@davinellulinvega I'll take you through the editorial process. I will clone your repo, make some tweaks and submit a PR.
@rougier Thank you very much for your feedback. I can confirm that figure 1 in the submitted paper is my own. Regarding the number of neural units per layer, you are correct that figure 1 in the original paper does contain the right amount per layer. However, nowhere are those numbers confirmed in the original text, hence my statement within the article. If you find it unfair, I can modify the sentence to better reflect reality.
In terms of license, is there any constraints that I should be aware of? Or would a CC-BY be enough?
@eroesch Thank you very much in advance for your help. I will keep an eye on my mail box for any notification.
@davinellulinvega You could add a simple sentence saying just that (the number of units might indicate in figure 1 but without confirmation anywhere in the text). For the code license, you can use https://choosealicense.com/. For Rescience C, we ask for open source license such that anybody can reuse your code. Typically, GPL v3 or BSD are fine.
@rougier Thank you very much for your answer. As per your advice, I have added a GPL v3 license to the source code, and modified the article's content to let the reader know of the situation regarding the number of neural units in the various layers.
In the next few days, I will also submit a request to Software Heritage to have the repository saved.
Thanks @davinellulinvega Looks like it is already archived in SH: https://archive.softwareheritage.org/browse/origin/directory/?origin_url=https://gitlab.com/davinellulinvega/armony.git
@eroesch You are entirely correct. A few days ago, I investigated the archival process. I expected to have to provide some proof or confirmation of my ownership of the git repository, before the project would be approved for recording. Instead, the request was processed in just a few moments.
However, since I have now added the License, I will request a new snapshot be created.