[REVIEW]: KinESim: Pre-equilibrium kinetic simulation of electrochemical reactions

[whedon]

Submitting author: @dap-biospec (Denis Proshlyakov)
Repository: https://github.com/dap-biospec/KinESim
Version: v1.6.8g
Editor: @kyleniemeyer
Reviewers: @usethedata, @FaustinCarter
Archive: 10.5281/zenodo.3375570

Status

Status badge code:

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

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

@usethedata & @FaustinCarter & @cfneese, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:

1. Make sure you're logged in to your GitHub account
2. 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 @kyleniemeyer know.

✨ Please try and complete your review in the next two weeks ✨

Review checklist for @usethedata

Conflict of interest

• As the reviewer I confirm that I have read the JOSS conflict of interest policy and that there are no conflicts of interest for me to review this work.

Code of Conduct

• I confirm that I read and will adhere to the JOSS code of conduct.

General checks

• Repository: Is the source code for this software available at the repository url?
• License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
• Version: Does the release version given match the GitHub release (v1.6.8g)?
• Authorship: Has the submitting author (@dap-biospec) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?

Functionality

• Installation: Does installation proceed as outlined in the documentation?
• Functionality: Have the functional claims of the software been confirmed?
• 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

• A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
• Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
• Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
• Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
• Automated tests: Are there automated tests or manual steps described so that the function of the software can be verified?
• 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

• Authors: Does the paper.md file include a list of authors with their affiliations?
• A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
• References: Do all archival references that should have a DOI list one (e.g., papers, datasets, software)?

Review checklist for @FaustinCarter

Conflict of interest

• As the reviewer I confirm that I have read the JOSS conflict of interest policy and that there are no conflicts of interest for me to review this work.

Code of Conduct

• I confirm that I read and will adhere to the JOSS code of conduct.

General checks

• Repository: Is the source code for this software available at the repository url?
• License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
• Version: Does the release version given match the GitHub release (v1.6.8g)?
• Authorship: Has the submitting author (@dap-biospec) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?

Functionality

• Installation: Does installation proceed as outlined in the documentation?
• Functionality: Have the functional claims of the software been confirmed?
• 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

• A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
• Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
• Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
• Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
• Automated tests: Are there automated tests or manual steps described so that the function of the software can be verified?
• 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

• Authors: Does the paper.md file include a list of authors with their affiliations?
• A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
• References: Do all archival references that should have a DOI list one (e.g., papers, datasets, software)?

Review checklist for @cfneese

Conflict of interest

• As the reviewer I confirm that I have read the JOSS conflict of interest policy and that there are no conflicts of interest for me to review this work.

Code of Conduct

• I confirm that I read and will adhere to the JOSS code of conduct.

General checks

• Repository: Is the source code for this software available at the repository url?
• License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
• Version: Does the release version given match the GitHub release (v1.6.8g)?
• Authorship: Has the submitting author (@dap-biospec) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?

Functionality

• Installation: Does installation proceed as outlined in the documentation?
• Functionality: Have the functional claims of the software been confirmed?
• 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

• A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
• Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
• Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
• Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
• Automated tests: Are there automated tests or manual steps described so that the function of the software can be verified?
• 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

• Authors: Does the paper.md file include a list of authors with their affiliations?
• A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
• References: Do all archival references that should have a DOI list one (e.g., papers, datasets, software)?

[whedon]

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @usethedata, @FaustinCarter, @cfneese it looks like you're currently assigned to review this paper 🎉.

⭐ Important ⭐

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:

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

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

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

@whedon commands

[whedon]

Attempting PDF compilation. Reticulating splines etc...

[whedon]

👉 Check article proof 📄 👈

[kyleniemeyer]

👋 @dap-biospec, @usethedata, @FaustinCarter, and @cfneese the actual review will take place in here. We don't have explicit deadlines for reviews, but it would be great if you all could provide your feedback / check off the boxes above within two or three weeks. If you need longer, just let me know—thanks!

[FaustinCarter]

I have a couple early comments about documentation:

1. I strongly recommend converting the file KES_Manual_API.docx to a reStructured text file so that it nicely renders on github. You should also take a subset of this documentation with installation instructions, etc, and put it in your README.rst file so it shows up on the github repo landing page. Docx is a proprietary file format that is not always readable by everyone, so it isn't really a suitable format for documentation. This is really easy to do with the rstdoc tool: https://pypi.org/project/rstdoc/.

2. As a followup to 1, you might consider using sphinx to generate documentation that the user can build, and that can optionally be hosted for free on a site like readthedocs.io. For an example of an Igor Pro related tool that does this, see: https://docs.byte-physics.de/igor-unit-testing-framework/. The rstdoc tool mentioned above will get you about 8/10 of the way there.

3. This Igor software is strongly tied to version 8 of Igor. This is pretty new and a lot of folks may not have updated from 7 (or, like me, from 6). Do you use features that are specific to version 8, or can this be relaxed somewhat? If it can't be relaxed, the installation instructions definitely need to state upfront that it requires version 8.

4. The existence of a pdf that overlaps with, but is not identical to, the docx file is confusing. You should have one source of truth for documentation.

OK, that's it for now. I'll have more comments as I work through testing the code. If you want help or advice for how to implement steps 1 and 2 above, I'm super happy to assist with that, but we should move that discussion to an issue in the KinESim repo.

[dap-biospec]

@FaustinCarter - thank you for recommendations! We will get started on them and I will reach out to you should we run into big problems. Much of this is new to us, but luckily it takes only once to learn.

[FaustinCarter]

@FaustinCarter - thank you for recommendations! We will get started on them and I will reach out to you should we run into big problems. Much of this is new to us, but luckily it takes only once to learn.

No worries! I just spent the last 10 minutes playing around with rstdoc, and it looks like you will get better conversion results if you spend a few minutes with your docx and apply styles (like heading 1, heading 2, normal text, etc) before you convert it (as opposed to just using bold, italics, and tabs to denote structure).

[dap-biospec]

@usethedata @FaustinCarter @cfneese @kyleniemeyer The first paper demonstrating application of this software has just been accepted. Submission files will be updated accordingly shortly.
https://pubs.acs.org/doi/10.1021/acs.analchem.9b00859

[dap-biospec]

@usethedata @FaustinCarter @cfneese @kyleniemeyer We updated documentation to comply with standards per @FaustinCarter recommendations. We are looking for feedback and recommendations for further improvements..

[kyleniemeyer]

Hi @usethedata @FaustinCarter and @cfneese, just wanted to check in on your reviews.

[FaustinCarter]

I've had a chance to look over the new documentation and it is really great! I have some further comments regarding installation and package organization:

The existence of the KES_complete.ipf file is confusing and provides a strong vector for unintended future version conflicts resulting from a developer updating a function in one of the sub-files and then forgetting to copy it to the right section of KES_complete.ipf. Instead, I think that a better solution is to create a KinESim_Procedures directory and a new procedure called KinESim.ipf. The new directory structure would be (note that KES_complete.ipf has been deleted):

KinESim_Procedures
|   KinESim.ipf
|
+---EChem
|       NPV_Illustrations.ipf
|       Spectroechem.ipf
|
\---KES
        KES_Common.ipf
        KES_Data.ipf
        KES_Panel.ipf
        KES_RKInt.ipf
        KES_Sets.ipf
        KES_Sim_Parallel.ipf
        KES_Sim_Sequential.ipf
        KES_Templates.ipf
        KES_Main.ipf

and the contents of KinESim.ipf would be:

#pragma rtGlobals=3
#pragma IgorVersion=8.0

#include  ":KES:KES_Main"
#include  ":EChem:NPV_Illustrations"
#include  ":EChem:Spectroechem"

Then the installation instructions could be simplified to: "Clone the git repo and open the KinESim.ipf in IgorPro". Now all your users (end or developer) are using the same set of files, and folks can operate directly out of the cloned git repository without needing to move a bunch of files around every time you update your code.

[FaustinCarter]

@kyleniemeyer I'd like to check off some boxes on the checklist for this review, but for some reason they are all grayed out. Is there a permissions issue or something? Thanks!

[FaustinCarter]

Regarding documentation: This is totally unnecessary in the sense that I'm not going to hold the review up over it, but it is good practice: I'd suggest putting all your documentation in a Docs folder in the repo (i.e. the .md files that aren't readme, and the Figures folder). This helps keep the repo clean and organized.

[kyleniemeyer]

@FaustinCarter sorry about that, I just added you as a collaborator on the joss-reviews repo, and if you accept you should be able to check the boxes. Not sure why those permissions didn't come when I added you as a reviewer.

[FaustinCarter]

@FaustinCarter sorry about that, I just added you as a collaborator on the joss-reviews repo, and if you accept you should be able to check the boxes. Not sure why those permissions didn't come when I added you as a reviewer.

So I clicked on the invitation and got the following: "Sorry, we couldn't find that repository invitation. It is possible that the invitation was revoked or that you are not logged into the invited account." Strange. I had the privs at one point (I remember accepting the original invitation).

[arfon]

Hi @FaustinCarter, apologies for causing you trouble here. It turns out there was a bug in our invite codes which didn't account for GitHub usernames with mixed cases (like yours). I've just fixed this an re-invited you to this repository (https://github.com/openjournals/joss-reviews/invitations).

My apologies again for any time wasted here but I believe this should now be fixed :-)

[dap-biospec]

@FaustinCarter Thanks for recommendations. The purpose of two versions is to keep work space clean for those who do not need development, but the point is taken. I will look in to moving internal code into an independent module, as we have done for G3F (https://github.com/dap-biospec/G3F). There, we have a single main code file while and all support code is loaded via includes and is hidden unless the user( developer) enables independent module development.

[FaustinCarter]

@dap-biospec Sounds good. I'm generally open to anything that doesn't require moving files around after the repository is cloned/downloaded.

[kyleniemeyer]

Hi @usethedata and @cfneese, could you submit your reviews within the next few days, or by the end of this week? Thanks!

[dap-biospec]

Hi @usethedata and @cfneese, We are finishing reorganization suggested by @FaustinCarter and will be uploading documentation and the restructured code on Monday am. This does not change functionality.

CC: kyleniemeyer

[dap-biospec]

@kyleniemeyer - I missed @ in the CC to the comment above.

[dap-biospec]

@usethedata, @FaustinCarter, @cfneese - all corrections suggested by @FaustinCarter have been made and merged. Code has been reorganized as described in the updated API document. Simulation algorithms remain unchanged (apart from operating now as an independent module).

Documentation and Demo files are collected in corresponding folders. We did not include two of the in the main KinESim.ipf (as suggested by @FaustinCarter) because they are particular illustrations used in the demo and not a common code.

We hope that repo is now in compliance with JOSS standards.

CC: @kyleniemeyer

[FaustinCarter]

@dap-biospec This looks great! So onto testing:

Ideally there would be some sort of test suite that runs some pre-configured simulations and checks to make sure that the output matches some pre-verified output. In the absence of that, running the demo and verifying that it works will probably be the best compromise (I'm not going to ask you to write a million unit tests).

I've tried loading up the Demo, and following the directions on the demo readme (which I think has an error: see the [insert image] text). When I open the demo I already have the full control panel there, so I think several of the steps in the instructions don't apply (i.e. the procedures are getting compiled on load already). So I click "do this sim" after I open the pxp, and I get an error "While executing a wave read, the following error occurred: index out of range". This is with the red arrow in data browser pointing to: "single analyte direct electrochemistry". If I drag the red arrow to a different folder and hit refresh in the KinESim window, I get a plot, but:

I don't really have any metrics for what is right, how to tell if the demo is doing what it is supposed to etc. It would be really helpful to include a step-by-step walk-through of doing a simple simulation with the demo, along with graphics showing expected outputs of the sim. Your interface is complex enough that a few helpful examples using the demo would go a long way towards building user-confidence that what they are getting output-wise is what they should get. To make sure that you have the same experience as your users, I'd download the pxp currently hosted on github and start from there.

I'm also totally lost with the procedures included in EChem and the panel that comes along for the ride under analysis. It is probably worth having separate API documentation for them, or a link to wherever that documentation exists if they were pulled in from another package. (Also, the Spectro E Chem panel doesn't seem to be closable by clicking on the red arrow, although maybe that is an Igor bug, as I'm noticing the same issue with the generated graph windows).

[dap-biospec]

@usethedata, @FaustinCarter, @cfneese we addressed all the issues that were raised by @FaustinCarter.

• We overhauled the demo and associated .md description. Demo now takes the user step by step through two simulation examples.
• The EChem unit is no longer included - most of the code was not related to KinESim and served more specialized purpose that would justify its inclusion here. The remaining code in NPV_Illustrations.ipf is only an illustration of how user-supplied functions can be used to set up and process the results. Common templates are described in the "Prototype functions ..." section of the API.
• Minor bugs that caused the reported error were fixed.
• We added a separate performance test experiment. It contains a pre-configured simulation and several graphs that provide visual reference of expected simulation results under various conditions.

CC: @kyleniemeyer

[kyleniemeyer]

@whedon check references

[whedon]

Attempting to check references...

[whedon]

OK DOIs

- 10.1021/acs.analchem.9b00859 is OK
- 10.1021/acs.jpcb.9b05866 is OK
- 10.1007/978-1-4419-6766-4_1 is OK
- 10.1007/s11047-019-09736-8 is OK

MISSING DOIs

- None

INVALID DOIs

- None

[kyleniemeyer]

@dap-biospec I made some minor edits to the paper, please merge these PRs: dap-biospec/KinESim#5 and dap-biospec/KinESim#6

[dap-biospec]

Thank you, @kyleniemeyer! Zenodo archive DOI is 10.5281/zenodo.3375570

[kyleniemeyer]

@whedon generate pdf

[whedon]

Attempting PDF compilation. Reticulating splines etc...

[whedon]

👉 Check article proof 📄 👈

[kyleniemeyer]

@whedon check references

[whedon]

Attempting to check references...

[whedon]

OK DOIs

- 10.1021/acs.analchem.9b00859 is OK
- 10.1021/acs.jpcb.9b05866 is OK
- 10.1007/978-1-4419-6766-4_1 is OK
- 10.1007/s11047-019-09736-8 is OK

MISSING DOIs

- None

INVALID DOIs

- None

[kyleniemeyer]

@whedon accept

[whedon]

No archive DOI set. Exiting...

[kyleniemeyer]

@whedon set 10.5281/zenodo.3375570 as archive

[whedon]

OK. 10.5281/zenodo.3375570 is the archive.

[kyleniemeyer]

@whedon accept

[whedon]

Attempting dry run of processing paper acceptance...

[whedon]

Check final proof 👉 openjournals/joss-papers#925

If the paper PDF and Crossref deposit XML look good in openjournals/joss-papers#925, then you can now move forward with accepting the submission by compiling again with the flag deposit=true e.g.

@whedon accept deposit=true

[kyleniemeyer]

@whedon accept deposit=true

[whedon]

Doing it live! Attempting automated processing of paper acceptance...

[whedon]

🐦🐦🐦 👉 Tweet for this paper 👈 🐦🐦🐦

[whedon]

🚨🚨🚨 THIS IS NOT A DRILL, YOU HAVE JUST ACCEPTED A PAPER INTO JOSS! 🚨🚨🚨

Here's what you must now do:

0. Check final PDF and Crossref metadata that was deposited 👉 Creating pull request for 10.21105.joss.01532 joss-papers#926
1. Wait a couple of minutes to verify that the paper DOI resolves https://doi.org/10.21105/joss.01532
2. If everything looks good, then close this review issue.
3. Party like you just published a paper! 🎉🌈🦄💃👻🤘

Any issues? notify your editorial technical team...

[kyleniemeyer]

Congrats @dap-biospec on your article's publication in JOSS! Many thanks to @usethedata and @FaustinCarter for reviewing.

[whedon]

🎉🎉🎉 Congratulations on your paper acceptance! 🎉🎉🎉

If you would like to include a link to your paper from your README use the following code snippets:

Markdown:
[![DOI](https://joss.theoj.org/papers/10.21105/joss.01532/status.svg)](https://doi.org/10.21105/joss.01532)

HTML:
<a style="border-width:0" href="https://doi.org/10.21105/joss.01532">
  <img src="https://joss.theoj.org/papers/10.21105/joss.01532/status.svg" alt="DOI badge" >
</a>

reStructuredText:
.. image:: https://joss.theoj.org/papers/10.21105/joss.01532/status.svg
   :target: https://doi.org/10.21105/joss.01532

This is how it will look in your documentation:

We need your help!

Journal of Open Source Software is a community-run journal and relies upon volunteer effort. If you'd like to support us please consider doing either one (or both) of the the following:

• Volunteering to review for us sometime in the future. You can add your name to the reviewer list here: https://joss.theoj.org/reviewer-signup.html
• Making a small donation to support our running costs here: https://numfocus.salsalabs.org/donate-to-joss