[REVIEW]: Learning Simulator: A simulation software for animal and human learning

whedon commented 3 years ago

Submitting author: @markusrobertjonsson (Markus Jonsson) Repository: https://github.com/learningsimulator/learningsimulator Version: v1.0.1 Editor: @oliviaguest Reviewers: @themkar, @CatarauCorina Archive: 10.5281/zenodo.4544535

:warning: JOSS reduced service mode :warning:

Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.

Status

Status badge code:

HTML: <a href="https://joss.theoj.org/papers/3987c9eb5e1e31414be3f320eed9303f"><img src="https://joss.theoj.org/papers/3987c9eb5e1e31414be3f320eed9303f/status.svg"></a>
Markdown: [![status](https://joss.theoj.org/papers/3987c9eb5e1e31414be3f320eed9303f/status.svg)](https://joss.theoj.org/papers/3987c9eb5e1e31414be3f320eed9303f)

Reviewers and authors:

Please avoid lengthy details of difficulties in the review thread. Instead, please create a new issue in the target repository and link to those issues (especially acceptance-blockers) by leaving comments in the review thread below. (For completists: if the target issue tracker is also on GitHub, linking the review thread in the issue or vice versa will create corresponding breadcrumb trails in the link target.)

Reviewer instructions & questions

@themkar & @CatarauCorina & @jkbye, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:

Make sure you're logged in to your GitHub account
Be sure to accept the invite at this URL: https://github.com/openjournals/joss-reviews/invitations

The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @oliviaguest know.

✨ Please start on your review when you are able, and be sure to complete your review in the next six weeks, at the very latest ✨

Review checklist for @themkar

Conflict of interest

[x] I confirm that I have read the JOSS conflict of interest (COI) policy and that: I have no COIs with reviewing this work or that any perceived COIs have been waived by JOSS for the purpose of this review.

Code of Conduct

[x] I confirm that I read and will adhere to the JOSS code of conduct.

General checks

[x] Repository: Is the source code for this software available at the repository url?
[x] License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
[x] Contribution and authorship: Has the submitting author (@markusrobertjonsson) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
[x] Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines

Functionality

[x] Installation: Does installation proceed as outlined in the documentation?
[x] Functionality: Have the functional claims of the software been confirmed?
[x] Performance: If there are any performance claims of the software, have they been confirmed? (If there are no claims, please check off this item.)

Documentation

[x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
[x] Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
[x] Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
[x] Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
[x] Automated tests: Are there automated tests or manual steps described so that the functionality of the software can be verified?
[x] Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support

Software paper

[x] Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
[x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
[x] State of the field: Do the authors describe how this software compares to other commonly-used packages?
[x] Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
[x] References: Is the list of references complete, and is everything cited appropriately that should be cited (e.g., papers, datasets, software)? Do references in the text use the proper citation syntax?

Review checklist for @CatarauCorina

Conflict of interest

[x] I confirm that I have read the JOSS conflict of interest (COI) policy and that: I have no COIs with reviewing this work or that any perceived COIs have been waived by JOSS for the purpose of this review.

Code of Conduct

[x] I confirm that I read and will adhere to the JOSS code of conduct.

General checks

[x] Repository: Is the source code for this software available at the repository url?
[x] License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
[x] Contribution and authorship: Has the submitting author (@markusrobertjonsson) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
[x] Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines

Functionality

[x] Installation: Does installation proceed as outlined in the documentation?
[x] Functionality: Have the functional claims of the software been confirmed?
[x] Performance: If there are any performance claims of the software, have they been confirmed? (If there are no claims, please check off this item.)

Documentation

[x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
[x] Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
[x] Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
[x] Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
[x] Automated tests: Are there automated tests or manual steps described so that the functionality of the software can be verified?
[x] Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support

Software paper

[x] Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
[x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
[x] State of the field: Do the authors describe how this software compares to other commonly-used packages?
[x] Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
[x] References: Is the list of references complete, and is everything cited appropriately that should be cited (e.g., papers, datasets, software)? Do references in the text use the proper citation syntax?

whedon commented 3 years ago

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @themkar, @CatarauCorina, @jkbye it looks like you're currently assigned to review this paper :tada:.

:warning: JOSS reduced service mode :warning:

Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.

:star: Important :star:

If you haven't already, you should seriously consider unsubscribing from GitHub notifications for this (https://github.com/openjournals/joss-reviews) repository. As a reviewer, you're probably currently watching this repository which means for GitHub's default behaviour you will receive notifications (emails) for all reviews 😿

To fix this do the following two things:

Set yourself as 'Not watching' https://github.com/openjournals/joss-reviews:

watching

You may also like to change your default settings for this watching repositories in your GitHub profile here: https://github.com/settings/notifications

notifications

For a list of things I can do to help you, just type:

@whedon commands

For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:

@whedon generate pdf

whedon commented 3 years ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

oliviaguest commented 3 years ago

Hi all! 👋 Thank you so much — @themkar, @CatarauCorina, and @jkbye — for accepting to review this. Please read the instructions above. Any questions, feedback on the paper, etc., please post here. Any very code-specific questions, suggestions, etc., please use the issues in the code repo and link to them from this thread so we can all keep track of them. 🌸

For an example of how this process plays out feel free to skim previous reviews, such as: #2285 and #2348. ☺️

oliviaguest commented 3 years ago

@whedon check references

whedon commented 3 years ago

Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1109/MCSE.2007.55 is OK

MISSING DOIs

- 10.1098/rsos.160734 may be a valid DOI for title: The power of associative learning and the ontogeny of optimal behaviour
- 10.1111/j.1751-0813.2010.00653.x may be a valid DOI for title: Carrots and sticks: Principles of animal training
- 10.1016/s0899-9007(98)00192-0 may be a valid DOI for title: Taste aversion learning: A contemporary perspective
- 10.4324/9780203803509 may be a valid DOI for title: Clinical applications of learning theory
- 10.1109/tsmcc.2012.2218595 may be a valid DOI for title: A survey of actor-critic reinforcement learning: Standard and natural policy gradients
- 10.1101/446906 may be a valid DOI for title: Social learning through associative processes: A computational theory
- 10.1098/rsos.180778 may be a valid DOI for title: What can associative learning do for planning?
- 10.1098/rsos.161011 may be a valid DOI for title: Memory for stimulus sequences: A divide between humans and other animals?
- 10.3758/brm.40.2.442 may be a valid DOI for title: HMS: A MATLAB simulator of the Harris model of associative learning
- 10.3758/lb.38.1.1 may be a valid DOI for title: An attention-modulated associative network
- 10.1016/j.cmpb.2012.02.004 may be a valid DOI for title: A Java simulator of Rescorla and Wagner’s prediction error model and configural cue extensions
- 10.3758/brm.41.1.29 may be a valid DOI for title: ALTSim: A MATLAB simulator for current associative learning theories
- 10.3758/brm.40.2.435 may be a valid DOI for title: Rapid-REM: A MATLAB simulator of the replaced-elements model
- 10.1109/adprl.2009.4927542 may be a valid DOI for title: A theoretical and empirical analysis of Expected Sarsa
- 10.1016/s0019-9958(77)90354-0 may be a valid DOI for title: An adaptive optimal controller for discrete-time Markov environments
- 10.3758/s13423-020-01749-0 may be a valid DOI for title: A-learning: A new formulation of associative learning theory

INVALID DOIs

- None

markusrobertjonsson commented 3 years ago

@oliviaguest missing DOIs addressed. If you need it, additional reviewer suggestions are satpreetsingh and seba-1511.

oliviaguest commented 3 years ago

@whedon check references

oliviaguest commented 3 years ago

@whedon generate pdf

whedon commented 3 years ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

oliviaguest commented 3 years ago

@whedon check references

whedon commented 3 years ago

Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1109/MCSE.2007.55 is OK
- 10.1098/rsos.160734 is OK
- 10.1111/j.1751-0813.2010.00653.x is OK
- 10.1016/s0899-9007(98)00192-0 is OK
- 10.4324/9780203803509 is OK
- 10.1109/tsmcc.2012.2218595 is OK
- 10.1101/446906 is OK
- 10.1098/rsos.180778 is OK
- 10.1098/rsos.161011 is OK
- 10.3758/brm.40.2.442 is OK
- 10.3758/lb.38.1.1 is OK
- 10.1016/j.cmpb.2012.02.004 is OK
- 10.3758/brm.41.1.29 is OK
- 10.3758/brm.40.2.435 is OK
- 10.1109/adprl.2009.4927542 is OK
- 10.1016/s0019-9958(77)90354-0 is OK
- 10.3758/s13423-020-01749-0 is OK

MISSING DOIs

- None

INVALID DOIs

- None

oliviaguest commented 3 years ago

@markusrobertjonsson great! 😊

oliviaguest commented 3 years ago

@themkar, @CatarauCorina, and @jkbye — can you please post a rough ETA for your reviews, please?

whedon commented 3 years ago

:wave: @jkbye, please update us on how your review is going.

whedon commented 3 years ago

:wave: @themkar, please update us on how your review is going.

whedon commented 3 years ago

:wave: @CatarauCorina, please update us on how your review is going.

CatarauCorina commented 3 years ago

@themkar, @CatarauCorina, and @jkbye — can you please post a rough ETA for your reviews, please?

Hi @oliviaguest by the end of this week for me, I was away, sorry for the late reply.

jkbye commented 3 years ago

Hello @oliviaguest thank you for your patience. I have been dealing with a family issue but will finish the review by next week.

themkar commented 3 years ago

Hello @oliviaguest, Happy new year. I shall be able to have this by 31st Jan. Hope this is ok.

oliviaguest commented 3 years ago

Great! Thanks @themkar, @CatarauCorina, and @jkbye! Just need an ETA for now, so this is all perfect! Hope you are all well. 🌼

oliviaguest commented 3 years ago

@whedon remind @themkar in a month

whedon commented 3 years ago

I don't recognize this description of time 'a' 'month '.

oliviaguest commented 3 years ago

@whedon remind @themkar in 1 month

whedon commented 3 years ago

Reminder set for @themkar in 1 month

oliviaguest commented 3 years ago

@whedon remind @CatarauCorina in 1 week

whedon commented 3 years ago

Reminder set for @CatarauCorina in 1 week

oliviaguest commented 3 years ago

@whedon remind @jkbye in 2 weeks

whedon commented 3 years ago

Reminder set for @jkbye in 2 weeks

whedon commented 3 years ago

:wave: @CatarauCorina, please update us on how your review is going.

CatarauCorina commented 3 years ago

Hi @oliviaguest , I am about to finish my review, but I have 1 question:

Any errors that I have encounter should I add them under the functionality observations?

Thank you for your help!

CatarauCorina commented 3 years ago

Functionality

✅ Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution. OK: A list of dependencies to install found in requirements.txt

✅ Functionality: Have the functional claims of the software been confirmed?

Yes, but whenever I press simulate and plot I am getting an additional window not shown in the documentation called “Plot and Export Progress”. This number of this windows increases with each simulation such that in the below image I have ran 3 simulations and therefore apart from my plot window is these 3 extra windows. Unsure weather this has to be addressed

- “ A stimulus is specified as a comma-separated list of stimulus elements, optionally with the intensity in brackets after the stimulus element name, for example e1,e2, or e1[0.3],e2,e3[0.9]” . However when modifying the default script to add intensity to the stimulus elements I get an error. Should the intensity be added in a different place ? And if so I think it's unclear in the documentation

Here is the head of the input file n_subjects = 10 mechanism = sr behaviors = response, no_response stimulus_elements = stimulus[0.9],background,reward[0.3] start_v = -1 alpha_v = 0.1 u = reward:10, default:0

“ n_subjects = n where n is a scalar expression evaluating to a positive integer”. When running python lesim.py gui and adding a value to n_subjects < 5 it fails, when running it in script mode it performs ok.

Documentation

✅ A statement of need Do the authors clearly state what problems the software is designed to solve and who the target audience is?

Yes, however it does not state the intended audience, I would add who the user persona is, in order to understand the background necessary to use it properly

✅ Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).

Yes, there is one example script for stimulus-response mechanism, though considering the complexity of the language a few more would be useful , for example one for every mechanism.

✅ Automated tests: Are there automated tests or manual steps described so that the functionality of the software can be verified?

Yes, but I couldn't find how to run all of them at once, and some seemed to fail as I ran them one by one

markusrobertjonsson commented 3 years ago

@oliviaguest I can respond to issues and questions raised by the reviewers as they come in, without having to wait for a final assessment, right? And I can make commits to source code and docs to address them, if needed?

oliviaguest commented 3 years ago

@markusrobertjonsson absolutely, yes!

whedon commented 3 years ago

:wave: @jkbye, please update us on how your review is going.

markusrobertjonsson commented 3 years ago

@CatarauCorina Thanks for reviewing! Below are my comments to the issues you raised.

Regarding the Plot/Export Progress: We have noticed a few window-related problems on Mac. This seems to be yet another one. The Progress dialog box just displays the progress bar and does not affect the functionality. I have now disabled it on Mac, and made an issue to look into this. To help with the debugging: Which OSX version are you using?
Regarding the problem with intensities: Stimulus intensities should not be specified in the parameter stimulus_elements, but in a @phase-block where it specifies the intensity of the stimulus element that is presented to the subject. The parameter stimulus_elements just declares the names of the elements. I have clarified the documentation.
Regarding the problem with n_subjects<5: As this does not seem to be a general problem, feel free to either send me the script where the error occured, and/or submit an issue.
Regarding the intended audience: In the README-file as well as in the Summary section on https://github.com/learningsimulator, it says "It is primarily intended for computational and behavioral biologists, ethologists, and psychologists, and it can also be useful to students and teachers in these fields." Let me know if you think this should be stated elsewhere as well.
Regarding example usage: Since changing the mechanism merely involves setting the value of the property mechanism (to ga, sr, es, ql, ac, or rw) while keeping the rest of the script intact, the complexity of the scripting language will not be showcased this way. Instead we refer to the script database on the project's website https://www.learningsimulator.org/script-database. I have added a reference to the database in the doc section "A Small Usage Example".
Regarding automated tests: There are automated tests (that are run by GitHub Actions), and the build-badge "Learning Simulator" shows whether or not they are passing. In a Python environment with pytest or unittest installed, the test suite can be run with python -m pytest (as in workflow.yml) or python -m unittest. To run an individual test file (for example test_beta.py that tests the script parameter beta), use python -m pytest tests/test_beta.py or python -m unittest tests/test_beta.py. (As far as I can see, the JOSS docs criteria for tests only says "An automated test suite hooked up to an external service such as Travis-CI or similar", not that there has to be instructions on how to manually execute the automated test suite.)

CatarauCorina commented 3 years ago

@markusrobertjonsson , thank you for your response !

10.14.5 is the version I have
Thank you for adding that to the documentation, is clear now , I have tested and it is working fine.
Ok, it's not a urgent issue, as I was saying in script mode is working fine, plus if I'm not mistaken usually in this type of experiments larger number of subjects are used.
That's fine - I might have missed it. Sorry !
That's perfect as the script database has 3 types of mechanisms, so that's great. Thanks!
You are correct, I didn't see there was a CI pipeline in place, I saw it now under actions and had a look at the stages of the build pipeline and you are correct they are running fine in that environment so it is surely something to do with my machine, which is not a requirement.

Thank you for clarifying my issues, I appreciate it. Good luck with your review further !

oliviaguest commented 3 years ago

@markusrobertjonsson feel free to make edits (as needed) before the other reviewers take a look.

markusrobertjonsson commented 3 years ago

@oliviaguest I have no additional edits to make at this point. I am looking forward to addressing possible further issues in the continued review process.

whedon commented 3 years ago

:wave: @themkar, please update us on how your review is going.

themkar commented 3 years ago

My apologies for the delay, I shall be able to complete it later this week.

oliviaguest commented 3 years ago

@themkar thank you for the update! 🎉

oliviaguest commented 3 years ago

@whedon re-invite @themkar as reviewer

whedon commented 3 years ago

OK, the reviewer has been re-invited.

@themkar please accept the invite by clicking this link: https://github.com/openjournals/joss-reviews/invitations

themkar commented 3 years ago

Hi @oliviaguest, I have finished my review and I am appending my comments (minor points) below. Many thanks.

comment #1 Functionality: Have the functional claims of the software been confirmed?

I have experimented with LeSIm using the GUI and the Computer Lab document and the three example scripts. I think it achieves the simplicity to which the authors refer to in the statement in need. The software offers good functionality for producing plots.

The GUI produced this warning in the chaining web, which can be explained better.

It would be good to have text processing functionalities under edit- copy, paste, find (or with keyboard shortcuts). Not sure if it is my system, but only undo and redo were available.

comment #2 Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.

Yes. Tried installation on Win, MacOS, Linux Mint. It might be worth including the possibility in which python is called with python3 (no alias).

comment #3 Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems). Yes. would be good if a sample script folder was incorporated in the GUI.

comment #4 Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)? Yes. And I especially liked the availability of a tutorial.

oliviaguest commented 3 years ago

@themkar I don't know if you want/need to try again with the image you tried to attach as I cannot see it..?

markusrobertjonsson commented 3 years ago

@themkar Thank you for reviewing! Below are my responses to your comments.

comment #1 The warning is a deprecated-syntax-warning. I have edited the script to use the updated syntax, so that the warning does not appear in that script anymore.

Additional text-processing functionalities are planned (see e.g. https://github.com/markusrobertjonsson/learningsimulator/issues/6), but since it is possible for the user to use his/her favourite editor and run scripts from a command line, we have not prioritized this.

comment #2 I have added python3 (and pip3) to the installation instructions.

comment #3 The addition of sample scripts (in a "Script Library") is a very good idea and is indeed planned for the future (https://github.com/markusrobertjonsson/learningsimulator/issues/26).

comment #4 Great!

oliviaguest commented 3 years ago

@markusrobertjonsson I think, if no other edits are obvious to you, we can continue with this without @jkbye (unless @jkbye you have time to take look within the next few days?), as I think he is very busy and might not have the time to check this out (totally understandable given pandemic).

themkar commented 3 years ago

@themkar I don't know if you want/need to try again with the image you tried to attach as I cannot see it..?

@oliviaguest is it visible now?

thanks @markusrobertjonsson for addressing my comments. Looking forward to using this software!

oliviaguest commented 3 years ago

@themkar yes, thank you! 🌼

markusrobertjonsson commented 3 years ago

@oliviaguest I have no other edits to make at this point. I very much appreciate the reviewer's comments.

openjournals / joss-reviews