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]: PySINDy: A Python package for the sparse identification of nonlinear dynamical systems from data #2104
Comments
Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @sixpearls, @dawbarton 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:
For a list of things I can do to help you, just type:
For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:
|
|
Hi @briandesilva et al, this is a very nice library! I checked most of the items in the list, but I did have a few comments, particularly on the unchecked items, below. AuthorshipI am not sure I see the SOFTWARE contribution of the last three authors, although it seems that based on the references these authors may have contributed to the theoretical development. In the past, I believe the JOSS policy was relatively strict about the software contribution of each author. Is that still the case? Could you please advise @terrytangyuan? Documentation:Statement of needThe documentation itself does not provide any statement of need or description of the mathematical formulation. I understand that the references that describe the problem are open-access, however in my experience there was a significant cognitive burden to understand the purpose of the library. These are my recommendations:
TestsI have checked it off because there does seem to be a thorough set of tests that cover the API, but it would improve the library to include regression tests that verify performance. For example, tests comparing the optimizers should show LASSO producing a larger number of terms than SR3 or STLSQ produces. |
Dear authors and reviewers We wanted to notify you that in light of the current COVID-19 pandemic, JOSS has decided to suspend submission of new manuscripts and to handle existing manuscripts (such as this one) on a "best efforts basis". We understand that you may need to attend to more pressing issues than completing a review or updating a repository in response to a review. If this is the case, a quick note indicating that you need to put a "pause" on your involvement with a review would be appreciated but is not required. Thanks in advance for your understanding. Arfon Smith, Editor in Chief, on behalf of the JOSS editorial team. |
Thanks, Arfon, for the update. At the moment we do not need to put a pause on our involvement in the review process. |
Hi @briandesilva, I will be happy to do a follow up review when you are ready. I will need guidance on the authorship question. @arfon could you advise on my authorship question?
|
@sixpearls - we generally try and steer clear of making authorship decisions. That said, we definitely do not require that all authors have to have contributed code/commits to the software as it's clearly possible for people to be important members of a project/collaboration without actually authoring the git commits. |
Thanks, @arfon. I must have been mis-remembering the policy. I'll check it off. |
@sixpearls, do you have any suggestions about how to render math in the README.rst file? I have drafted a simple mathematical description and some accompanying text for the README, but I have so far been unable to get any of the math to be rendered on GitHub. After some poking around other repositories, the only solution I have been able to find is to export the rendered equations as images, then embed the images in the README (something like what this repository does). This would work for users viewing the README on GitHub, but I'm not sure whether it would work for those viewing the package on PyPI. Any recommendations you can provide on how to proceed would be greatly appreciated. |
@briandesilva I can't find a better example, but I was imagining just a simple ASCII formulation like this one. You could alternatively describe the mathematical concepts "in" Python or pseudocode. That said, Your update to the "original paper" notebook is exactly what I had in mind. Since your API is fairly semantic, I think a brief prose description with a well commented example and a link to the "main idea" section of this notebook (or breaking it out onto its own page) would be sufficient for the readme and PyPi long description. |
I believe the PyPI long description can render images, but you would need to find a host to serve the graphic to PyPI and I'm not sure if GitHub does that. So, rather than using plots in the PyPI long description, you could do something like show the output of numpy's |
@sixpearls, I've added some information on the mathematics behind SINDy and a short example to the README (and the PyPI description). I also took your advice and moved the introduction to SINDY to its own notebook. Please take a look when you get a chance. |
Hi @briandesilva, this looks great. I will check off the item from my review and recommend that this can be published. This is a great library, congratulations! |
👋 @dawbarton - how are you getting on with your review? |
Apologies for the delay, life has been a bit manic (and I also missed the ping somehow). I have been through the package, documentation, and paper - it looks good overall. A few comments: When I run the example in the README, I don't get the specified output. It's close but not quite the same; see below.
Syntax highlighting in the docs is a little off - seems to get confused by ' characters. Docs are relatively clear but slightly odd in places. For example, in the pysindy.feature_library package, the first class mentions things like The extensive examples are very nice. If only all packages had so many! Overall, this is a great package. Thanks for putting it together. |
Thanks for your feedback!
|
Equation formatting in the readme is fixed now. The code-blocks were automatically interpreted as python and hence the funny looking syntax highlighting. |
@dawbarton, we have updated the package and documentation to address each of your comments. When you get a chance could you please take another look to make sure everything meets your standards? |
I haven't checked whether you've fixed the regression with the readme example but everything else looks good to me. 👍 |
Great! Is there anything else we need to do to move things forward? |
Sorry for the delay here @briandesilva - the handling editor (@terrytangyuan) has limited availability to edit right now but I can help moving this forward. |
@whedon generate pdf |
@whedon check references |
|
@briandesilva - At this point could you make a new release of this software that includes the changes that have resulted from this review. Then, please make an archive of the software in Zenodo/figshare/other service and update this thread with the DOI of the archive? For the Zenodo/figshare archive, please make sure that:
I can then move forward with accepting the submission. |
@arfon Thanks. Please continue taking it from here since it will be ready for handing over to you anyways soon for final acceptance and deposit. |
@arfon - I have made a new release including all the changes prompted by this review. Here is the Zenodo DOI for PySINDy: |
@whedon set 10.5281/zenodo.3832319 as archive |
OK. 10.5281/zenodo.3832319 is the archive. |
@whedon accept |
|
|
👋 @openjournals/joss-eics, this paper is ready to be accepted and published. Check final proof 👉 openjournals/joss-papers#1450 If the paper PDF and Crossref deposit XML look good in openjournals/joss-papers#1450, then you can now move forward with accepting the submission by compiling again with the flag
|
@whedon accept deposit=true |
|
🐦🐦🐦 👉 Tweet for this paper 👈 🐦🐦🐦 |
🚨🚨🚨 THIS IS NOT A DRILL, YOU HAVE JUST ACCEPTED A PAPER INTO JOSS! 🚨🚨🚨 Here's what you must now do:
Any issues? notify your editorial technical team... |
@sixpearls, @dawbarton - many thanks for your reviews here and to @terrytangyuan for editing this submission ✨ @briandesilva - your paper is now accepted into JOSS ⚡🚀💥 |
🎉🎉🎉 Congratulations on your paper acceptance! 🎉🎉🎉 If you would like to include a link to your paper from your README use the following code snippets:
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:
|
Submitting author: @briandesilva (Brian de Silva)
Repository: https://github.com/dynamicslab/pysindy
Version: v0.12.0
Editor: @terrytangyuan
Reviewer: @sixpearls, @dawbarton
Archive: 10.5281/zenodo.3832319
Status
Status badge code:
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
@sixpearls & @dawbarton, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:
The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @terrytangyuan know.
✨ Please try and complete your review in the next two weeks ✨
Review checklist for @sixpearls
Conflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
Review checklist for @dawbarton
Conflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
The text was updated successfully, but these errors were encountered: