Skip to content
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]: SysIdentPy: A Python package for System Identification #2384

Closed
38 tasks done
whedon opened this issue Jun 24, 2020 · 67 comments
Closed
38 tasks done

[REVIEW]: SysIdentPy: A Python package for System Identification #2384

whedon opened this issue Jun 24, 2020 · 67 comments
Assignees
Labels
accepted published Papers published in JOSS recommend-accept Papers recommended for acceptance in JOSS. review

Comments

@whedon
Copy link

whedon commented Jun 24, 2020

Submitting author: @wilsonrljr (Wilson Rocha Lacerda Junior)
Repository: https://github.com/wilsonrljr/sysidentpy
Version: 0.1.2
Editor: @dpsanders
Reviewers: @Shibabrat, @dawbarton
Archive: 10.5281/zenodo.4026516

⚠️ JOSS reduced service mode ⚠️

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

Status badge code:

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

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

@Shibabrat and @dawbarton, 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 @dpsanders know.

Please try and complete your review in the next six weeks

Review checklist for @Shibabrat

Conflict of interest

  • 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

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?
  • Contribution and authorship: Has the submitting author (@wilsonrljr) 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 functionality 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

  • Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
  • A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • State of the field: Do the authors describe how this software compares to other commonly-used packages?
  • Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
  • 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 @dawbarton

Conflict of interest

  • 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

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?
  • Contribution and authorship: Has the submitting author (@wilsonrljr) 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 functionality 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

  • Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
  • A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • State of the field: Do the authors describe how this software compares to other commonly-used packages?
  • Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
  • 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
Copy link
Author

whedon commented Jun 24, 2020

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

⚠️ JOSS reduced service mode ⚠️

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.

⭐ 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:

watching

  1. 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
Copy link
Author

whedon commented Jun 24, 2020

@whedon
Copy link
Author

whedon commented Jun 24, 2020

Reference check summary:

OK DOIs

- None

MISSING DOIs

- https://doi.org/10.1016/0005-1098(89)90019-8 may be missing for title: System identification: theory for the user
- https://doi.org/10.1080/00207178908559683 may be missing for title: Representations of non-linear systems: the NARMAX model
- https://doi.org/10.1016/j.ymssp.2015.12.031 may be missing for title: Sufficient conditions for rate-independent hysteresis in autoregressive identified models
- https://doi.org/10.1109/access.2019.2926057 may be missing for title: Control of Hysteretic Systems Through an Analytical Inverse Compensation Based on a NARX Model
- https://doi.org/10.1016/s0307-904x(03)00071-4 may be missing for title: Modelling and prediction of machining errors using ARMAX and NARMAX structures
- https://doi.org/10.1016/j.neucom.2015.08.022 may be missing for title: Ultra-orthogonal forward regression algorithms for the identification of non-linear dynamic systems
- https://doi.org/10.1109/tbme.2002.803507 may be missing for title: NARMAX representation and identification of ankle dynamics
- https://doi.org/10.1016/b978-0-12-811788-0.00008-1 may be missing for title: Applications of NARMAX in Space Weather

INVALID DOIs

- None

@dpsanders
Copy link

@whedon add @dawbarton as reviewer

@whedon whedon assigned dpsanders and Shibabrat and unassigned dpsanders Jun 24, 2020
@whedon
Copy link
Author

whedon commented Jun 24, 2020

OK, @dawbarton is now a reviewer

@dpsanders dpsanders reopened this Jun 24, 2020
@dpsanders
Copy link

👋 @arfon, I seem to have messed up the reviewer assignment, sorry. @dawbarton doesn't seem to appear as a reviewer. Should I fix this by hand?

@arfon
Copy link
Member

arfon commented Jun 24, 2020

👋 @arfon, I seem to have messed up the reviewer assignment, sorry. @dawbarton doesn't seem to appear as a reviewer. Should I fix this by hand?

@dpsanders - Yes, I'm afraid Whedon doesn't yet know how to add a checklist for a reviewer once the review has started.

You can do this by editing the body of the issue at the top of this thread. Let me know if you need my help with this.

@dpsanders
Copy link

@arfon OK thanks, done.

@dawbarton
Copy link

Overall, looks good. It installs easily and usage is as described. Documentation is good overall.

A few detailed comments below.

  • Paper - some comments scribbled onto the PDF 10.21105.joss.02384 1.pdf; it's a bit sloppy in places and looks like it really needed proof reading again.
  • Documentation
    • introduction_to_narmax.html: $F^l$ in the "So, what is a NARMAX model?" section appears to be slightly misleading. The notation seems to indicate a polynomial function (particularly with the comment about setting $l=1$ recovering the ARMAX model) whereas you describe it as a general nonlinear function. In the section that follows, you refer to "unknown mapping $f[⋅]$"; I presume this was the $F^l$ in the previous section.
    • user_guide.html: the formatting of the text goes awry about halfway down (all the text appears to turn into headings). The text is fine, though you might want to consider how it is structured. I would recommend putting the function you are describing first and then explain it, rather than give a (rather specific) explanation and then the function. For example, the get_siso_data function - you start by giving an equation then say that color_noise equal to True means that you get a certain noise relationship. In the context of the equation (alone) that statement is confusing - what has color_noise got to do with the equation? It doesn't have any variables called color_noise in it. It's not until the end of the section that you see that the text was actually talking about a particular function (that is, get_siso_data) to generate sample data.
    • I'm not sure what the Jupyter notebooks section of the docs is supposed to do - it appears to be largely the same as the user guide.
  • I can't see any automated tests in the repo - have I missed them or do they not exist? I think JOSS requires them?

@wilsonrljr
Copy link

Dear @dawbarton , we appreciate the time and effort that you dedicated to providing feedback on our manuscript.

  • Paper - some comments scribbled onto the PDF 10.21105.joss.02384 1.pdf; it's a bit sloppy in places and looks like it really needed proof reading again.
    Regarding the manuscript, we are incorporating the suggestions made by you. The comments will result in valuable improvements to our paper.

  • introduction_to_narmax.html: $F^l$ in the "So, what is a NARMAX model?" section appears to be slightly misleading. The notation seems to indicate a polynomial function (particularly with the comment about setting $l=1$ recovering the ARMAX model) whereas you describe it as a general nonlinear function. In the section that follows, you refer to "unknown mapping $f[⋅]$"; I presume this was the $F^l$ in the previous section.
    Thank you for pointing this out. The $F$ in NARMAX indicates a general nonlinear function (Neural, Fuzzy, and many other). However, because we define it as a polynomial function, we have to keep the definition consistent. I will upload the changes soon.

  • user_guide.html: the formatting of the text goes awry about halfway down (all the text appears to turn into headings). The text is fine, though you might want to consider how it is structured.
    We agree with you. The documentation is in a very early stage. Actually, the website version will be replaced very soon. Many issues with structure and formatting is due the current way it is done. We are converting the docs to a properly website.

In this case, I have a question: Would these changes concerning the documentation in the website be mandatory in this revision process?

  • I'm not sure what the Jupyter notebooks section of the docs is supposed to do - it appears to be largely the same as the user guide.
    Each notebook presents a different exemple of how particular parameters works and some tips. We made this section for the users who have no prior knowledge of the library or NARMAX models. There are particular usage of some parameters we didn't show in user guide.

  • I can't see any automated tests in the repo - have I missed them or do they not exist? I think JOSS requires them?
    We have tests that coverage approximately 87% of the our functions. These tests are placed on "tests" folder inside each class path.

@wilsonrljr
Copy link

@whedon generate pdf

@whedon
Copy link
Author

whedon commented Jul 29, 2020

@wilsonrljr
Copy link

@whedon generate pdf

@whedon
Copy link
Author

whedon commented Jul 29, 2020

@wilsonrljr
Copy link

Dear @dawbarton, the paper was updated following your suggestions. if you want to add any comments, just let me know.

Regarding my previous comment regarding the documentation, please let me know if the those suggestions would be mandatory. I am working on moving the documentation to a tool other than Sphinx. However, if corrections to the web documentation are necessary I will update the current files.

@dawbarton
Copy link

All looks good to me now. I've ticked off all the items on the checklist.

@dpsanders
Copy link

Thanks a lot @dawbarton!

@Shibabrat Just checking in to see how progress is with your review?

@dpsanders
Copy link

@wilsonrljr: I was wondering if http://www.nonlinearbenchmark.org/ is relevant?

@wilsonrljr
Copy link

Thank you, @dawbarton!

@dpsanders Regarding the that site, it is really good! It contains a set of really great data for system identification. I have an exemple here for the F-16 dataset (since I have used it in my dissertation). I'll add an notebook in example folder later today. I have other examples with real world data used as benchmark for SI already implemented and I'll upload them in the next update of the package. However, I'll add the F-16 example later today because those data are really good.

@dawbarton
Copy link

Regarding http://www.nonlinearbenchmark.org/ I should mention that it's quite a nice meeting that runs each year and is directly relevant to your software package; it's well worth attending if the situation ever allows. (I went a few years ago.)

@wilsonrljr
Copy link

Thanks for pointing that out, dawbarton. I worked with some of those datasets on my master dissertation. Some of the authors collaborating with the website are reference on system identification using NARMAX models. It's really important to share our work with researchers on the field with examples that make sense for them.

I updated the master branch with the F-16 example. It's a simple showcase for now, without any comparison with other works. https://wilsonrljr.github.io/sysidentpy/examples/f_16_benchmark.html

@dpsanders
Copy link

@whedon accept

@whedon
Copy link
Author

whedon commented Sep 27, 2020

Attempting dry run of processing paper acceptance...

@whedon whedon added the recommend-accept Papers recommended for acceptance in JOSS. label Sep 27, 2020
@whedon
Copy link
Author

whedon commented Sep 27, 2020

👋 @openjournals/joss-eics, this paper is ready to be accepted and published.

Check final proof 👉 openjournals/joss-papers#1763

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

@whedon accept deposit=true

@whedon
Copy link
Author

whedon commented Sep 27, 2020

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

OK DOIs

- None

MISSING DOIs

- 10.1016/0005-1098(89)90019-8 may be a valid DOI for title: System identification: theory for the user
- 10.1080/00207178908559683 may be a valid DOI for title: Representations of non-linear systems: the NARMAX model
- 10.1016/j.ymssp.2015.12.031 may be a valid DOI for title: Sufficient conditions for rate-independent hysteresis in autoregressive identified models
- 10.1109/access.2019.2926057 may be a valid DOI for title: Control of Hysteretic Systems Through an Analytical Inverse Compensation Based on a NARX Model
- 10.1016/s0307-904x(03)00071-4 may be a valid DOI for title: Modelling and prediction of machining errors using ARMAX and NARMAX structures
- 10.1016/j.neucom.2015.08.022 may be a valid DOI for title: Ultra-orthogonal forward regression algorithms for the identification of non-linear dynamic systems
- 10.1109/tbme.2002.803507 may be a valid DOI for title: NARMAX representation and identification of ankle dynamics
- 10.1016/b978-0-12-811788-0.00008-1 may be a valid DOI for title: Applications of NARMAX in Space Weather
- 10.1016/j.automatica.2020.109099 may be a valid DOI for title: A Tree Adjoining Grammar Representation for Models Of Stochastic Dynamical Systems

INVALID DOIs

- None

@dpsanders
Copy link

@wilsonrljr Could you please fix the missing DOIs identified above? Thanks!

@wilsonrljr
Copy link

@dpsanders I've fixed the missing DOIs.

@dpsanders
Copy link

@whedon accept

@whedon
Copy link
Author

whedon commented Sep 28, 2020

Attempting dry run of processing paper acceptance...

@whedon
Copy link
Author

whedon commented Sep 28, 2020

👋 @openjournals/joss-eics, this paper is ready to be accepted and published.

Check final proof 👉 openjournals/joss-papers#1765

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

@whedon accept deposit=true

@whedon
Copy link
Author

whedon commented Sep 28, 2020

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

OK DOIs

- 10.1016/0005-1098(89)90019-8 is OK
- 10.1080/00207178908559683 is OK
- 10.1016/j.ymssp.2015.12.031 is OK
- 10.1109/access.2019.2926057 is OK
- 10.1016/s0307-904x(03)00071-4 is OK
- 10.1016/j.neucom.2015.08.022 is OK
- 10.1109/tbme.2002.803507 is OK
- 10.1016/b978-0-12-811788-0.00008-1 is OK
- 10.1016/j.automatica.2020.109099 is OK

MISSING DOIs

- None

INVALID DOIs

- None

@Kevin-Mattheus-Moerman
Copy link
Member

Kevin-Mattheus-Moerman commented Oct 1, 2020

@wilsonrljr I've reviewed your paper and ask you to make these last minor changes:

  • Please add a country to your affiliation

  • The figure axis font for figure 1 is very small for the bottom figure. I recommend you increase the font and update the picture.

  • Please edit the ZENODO archive meta data to match that of the paper, i.e. the title and author list has to match that of the paper. (@dpsanders for future reference please check this before proceeding to recommend acceptance, thanks. )

  • Your paper is about to be processed for acceptance. Please thoroughly read through it once more yourself. .

Let me know when you've made these changes. Thanks.

Kevin Moerman
AEiC JOSS

@wilsonrljr
Copy link

@whedon generate pdf

@whedon
Copy link
Author

whedon commented Oct 2, 2020

👉📄 Download article proof 📄 View article proof on GitHub 📄 👈

@wilsonrljr
Copy link

@Kevin-Mattheus-Moerman Thank you for reviewing the paper.

  • I added a country affiliation;
  • I've increased the font size of the figure 1. Now all fonts have the same size on labels;
  • I updated the title on Zenodo metadata to match the paper title. The author list was already the same of the paper, but I've included the country affiliation on Zenodo too.

Let me know if you need anything else.

@Kevin-Mattheus-Moerman
Copy link
Member

@whedon accept

@whedon
Copy link
Author

whedon commented Oct 2, 2020

Attempting dry run of processing paper acceptance...

@whedon
Copy link
Author

whedon commented Oct 2, 2020

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

OK DOIs

- 10.1016/0005-1098(89)90019-8 is OK
- 10.1080/00207178908559683 is OK
- 10.1016/j.ymssp.2015.12.031 is OK
- 10.1109/access.2019.2926057 is OK
- 10.1016/s0307-904x(03)00071-4 is OK
- 10.1016/j.neucom.2015.08.022 is OK
- 10.1109/tbme.2002.803507 is OK
- 10.1016/b978-0-12-811788-0.00008-1 is OK
- 10.1016/j.automatica.2020.109099 is OK

MISSING DOIs

- None

INVALID DOIs

- None

@whedon
Copy link
Author

whedon commented Oct 2, 2020

👋 @openjournals/joss-eics, this paper is ready to be accepted and published.

Check final proof 👉 openjournals/joss-papers#1772

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

@whedon accept deposit=true

@Kevin-Mattheus-Moerman
Copy link
Member

@whedon accept deposit=true

@whedon
Copy link
Author

whedon commented Oct 2, 2020

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

@whedon whedon added accepted published Papers published in JOSS labels Oct 2, 2020
@whedon
Copy link
Author

whedon commented Oct 2, 2020

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

@whedon
Copy link
Author

whedon commented Oct 2, 2020

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

Here's what you must now do:

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

Any issues? Notify your editorial technical team...

@whedon
Copy link
Author

whedon commented Oct 2, 2020

🎉🎉🎉 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.02384/status.svg)](https://doi.org/10.21105/joss.02384)

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

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

This is how it will look in your documentation:

DOI

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:

@Kevin-Mattheus-Moerman
Copy link
Member

Thanks for your review efforts @Shibabrat, @dawbarton!! And thanks @dpsanders for editing this one!

Congratulations on the paper @wilsonrljr ! 🎉

@dpsanders
Copy link

dpsanders commented Oct 2, 2020

Thanks @Kevin-Mattheus-Moerman.

Congratulations @wilsonrljr on your nice paper!

And many thanks to @dawbarton and @Shibabrat for your hard work on the reviews!

@wilsonrljr
Copy link

Many thanks @dpsanders, @dawbarton, @Shibabrat and @Kevin-Mattheus-Moerman! All of you have contributed to the effort to obtain the this version of the paper.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted published Papers published in JOSS recommend-accept Papers recommended for acceptance in JOSS. review
Projects
None yet
Development

No branches or pull requests

7 participants