openjournals / joss-reviews

Reviews for the Journal of Open Source Software
Creative Commons Zero v1.0 Universal
708 stars 38 forks source link

[REVIEW]: PyKoopman: A Python Package for Data-Driven Approximation of the Koopman Operator #5881

Closed editorialbot closed 7 months ago

editorialbot commented 12 months ago

Submitting author: !--author-handle-->@pswpswpsw<!--end-author-handle-- (Shaowu Pan) Repository: https://github.com/dynamicslab/pykoopman Branch with paper.md (empty if default branch): Version: v1.1.0 Editor: !--editor-->@olexandr-konovalov<!--end-editor-- Reviewers: @ulf1, @fandreuz Archive: 10.5281/zenodo.10685233

Status

status

Status badge code:

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

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

@ulf1 & @fandreuz, your review will be checklist based. Each of you will have a separate checklist that you should update when carrying out your review. First of all you need to run this command in a separate comment to create the checklist:

@editorialbot generate my checklist

The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @olexandr-konovalov 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

Checklists

📝 Checklist for @fandreuz

📝 Checklist for @ulf1

olexandr-konovalov commented 7 months ago

@editorialbot generate pdf

editorialbot commented 7 months ago

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

olexandr-konovalov commented 7 months ago

@pswpswpsw thank you - I've printed and proofread it (sorry took a while to reach it), and I like the current shape of the article, but here there are some, hopefully final, suggestions:

pswpswpsw commented 7 months ago

@olexandr-konovalov Thank you Editor. I have corrected all of your suggestions except the first one since I do not have that figure editable. Thank you for your time editing our paper!

pswpswpsw commented 7 months ago

@editorialbot generate pdf

editorialbot commented 7 months ago

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

editorialbot commented 7 months ago

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

olexandr-konovalov commented 7 months ago

@pswpswpsw thanks! I hope we can now progress quickly.

One more instance of koopman starting in lowercase - page 8, line 236.

When fixed, please make a new release and archive it on Zenodo, and let me know.

Archiving on Zenodo, you may have to edit your archive metadata, to change "dynamicslab/pykoopman: v1.0.9" to "PyKoopman: A Python Package for Data-Driven Approximation of the Koopman Operator" to match the title of the paper - compare how this is done in any of the recently published JOSS papers.

olexandr-konovalov commented 7 months ago

@pswpswpsw actually, a new release is only required if additional changes to the software were made - you don't have to make a release, but just update the metadata, to make the title matching (thanks @kyleniemeyer for the clarification)

pswpswpsw commented 7 months ago

@olexandr-konovalov just finished editing and updated the newest paper. Also, the zenodo is fixed with the right title. https://zenodo.org/records/10684387

pswpswpsw commented 7 months ago

@editorialbot generate pdf

editorialbot commented 7 months ago

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

olexandr-konovalov commented 7 months ago

@pswpswpsw after all, we might need a new release - https://github.com/dynamicslab/pykoopman/tree/v1.0.9 has a lot of files ending with ~ which seem to be added accidentally. Sorry, haven't noticed before!

pswpswpsw commented 7 months ago

@olexandr-konovalov no worries. here is it! https://zenodo.org/records/10685233

olexandr-konovalov commented 7 months ago

The repository looks good. Next, https://zenodo.org/records/10685233 says v1.1.0, but surprisingly it sats in two places

Have you set up everything like https://docs.github.com/en/repositories/archiving-a-github-repository/referencing-and-citing-content suggests? If so, that's puzzling to see. Anyhow, perhaps this is something you can fix on Zenodo manually?

pswpswpsw commented 7 months ago

@olexandr-konovalov Just fixed! Sorry for the oversight. I just manually fixed that. The issue is I tried to use zenodo meta file to avoid filling. However, it has some buggy issues.

olexandr-konovalov commented 7 months ago

@editorialbot set 10.5281/zenodo.10685233 as archive

editorialbot commented 7 months ago

Done! archive is now 10.5281/zenodo.10685233

olexandr-konovalov commented 7 months ago

@editorialbot set v1.1.0 as version

editorialbot commented 7 months ago

Done! version is now v1.1.0

olexandr-konovalov commented 7 months ago

@editorialbot generate pdf

olexandr-konovalov commented 7 months ago

@editorialbot check references

editorialbot commented 7 months ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1017/S0022112010001217 is OK
- 10.1137/1.9781611974508 is OK
- 10.5281/zenodo.3828935 is OK
- 10.21105/joss.03994 is OK
- 10.1146/annurev-fluid-030121-015835 is OK
- 10.1016/j.arcontrol.2009.12.001 is OK
- 10.1007/978-0-387-40065-5 is OK
- 10.21105/joss.00530 is OK
- 10.21105/joss.03994 is OK
- 10.1017/9781108380690 is OK
- 10.1137/21M1401243 is OK
- 10.1063/1.4772195 is OK
- 10.1007/s00332-017-9437-7 is OK
- 10.1146/annurev-fluid-011212-140652 is OK
- 10.1063/1.4993854 is OK
- 10.1007/BF02532251 is OK
- 10.1038/s41467-017-00030-8 is OK
- 10.1007/978-3-662-04323-3_15 is OK
- 10.1017/S0022112009992059 is OK
- 10.1002/9781118535561 is OK
- 10.48550/arXiv.1710.09668 is OK
- 10.1137/18M1225409 is OK
- 10.1063/1.5011399 is OK
- 10.1038/s41467-017-02388-1 is OK
- 10.1098/rspa.2017.0844 is OK
- 10.1103/PhysRevLett.120.024102 is OK
- 10.1137/19M1274067 is OK
- 10.1016/j.jcp.2018.10.045 is OK
- 10.1073/pnas.1906995116 is OK
- 10.1126/science.aaw4741 is OK
- 10.1007/s00466-019-01711-5 is OK
- 10.1137/130932715 is OK
- 10.1016/j.cma.2016.03.025 is OK
- 10.1016/j.physd.2020.132401 is OK
- 10.1073/pnas.1118984109 is OK
- 10.1073/pnas.1620045114 is OK
- 10.1073/pnas.0609476104 is OK
- 10.1126/science.1165893 is OK
- 10.1038/ncomms9133 is OK
- 10.1073/pnas.1517384113 is OK
- 10.1007/s00332-015-9258-5 is OK
- 10.1126/sciadv.1602614 is OK
- 10.3934/jcd.2015005 is OK
- 10.1038/s41467-018-07210-0 is OK
- 10.1137/18M1177846 is OK
- 10.48550/arXiv.1710.04340 is OK
- 10.1017/jfm.2021.271 is OK
- 10.1016/j.ifacol.2016.10.250 is OK
- 10.1109/TAC.2020.2978039 is OK
- 10.1007/978-3-030-35713-9 is OK
- 10.1088/2632-2153/abf0f5 is OK
- 10.1016/j.automatica.2019.05.016 is OK
- 10.1137/20M1325678 is OK
- 10.21105/joss.02104 is OK
- 10.1137/17M115414X is OK
- 10.1016/j.physd.2004.06.015 is OK
- 10.1137/19M1267246 is OK
- 10.1137/15M1013857 is OK
- 10.1088/2632-2153/ac3de0 is OK

MISSING DOIs

- None

INVALID DOIs

- None
editorialbot commented 7 months ago

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

olexandr-konovalov commented 7 months ago

@editorialbot recommend-accept

editorialbot commented 7 months ago
Attempting dry run of processing paper acceptance...
editorialbot commented 7 months ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1017/S0022112010001217 is OK
- 10.1137/1.9781611974508 is OK
- 10.5281/zenodo.3828935 is OK
- 10.21105/joss.03994 is OK
- 10.1146/annurev-fluid-030121-015835 is OK
- 10.1016/j.arcontrol.2009.12.001 is OK
- 10.1007/978-0-387-40065-5 is OK
- 10.21105/joss.00530 is OK
- 10.21105/joss.03994 is OK
- 10.1017/9781108380690 is OK
- 10.1137/21M1401243 is OK
- 10.1063/1.4772195 is OK
- 10.1007/s00332-017-9437-7 is OK
- 10.1146/annurev-fluid-011212-140652 is OK
- 10.1063/1.4993854 is OK
- 10.1007/BF02532251 is OK
- 10.1038/s41467-017-00030-8 is OK
- 10.1007/978-3-662-04323-3_15 is OK
- 10.1017/S0022112009992059 is OK
- 10.1002/9781118535561 is OK
- 10.48550/arXiv.1710.09668 is OK
- 10.1137/18M1225409 is OK
- 10.1063/1.5011399 is OK
- 10.1038/s41467-017-02388-1 is OK
- 10.1098/rspa.2017.0844 is OK
- 10.1103/PhysRevLett.120.024102 is OK
- 10.1137/19M1274067 is OK
- 10.1016/j.jcp.2018.10.045 is OK
- 10.1073/pnas.1906995116 is OK
- 10.1126/science.aaw4741 is OK
- 10.1007/s00466-019-01711-5 is OK
- 10.1137/130932715 is OK
- 10.1016/j.cma.2016.03.025 is OK
- 10.1016/j.physd.2020.132401 is OK
- 10.1073/pnas.1118984109 is OK
- 10.1073/pnas.1620045114 is OK
- 10.1073/pnas.0609476104 is OK
- 10.1126/science.1165893 is OK
- 10.1038/ncomms9133 is OK
- 10.1073/pnas.1517384113 is OK
- 10.1007/s00332-015-9258-5 is OK
- 10.1126/sciadv.1602614 is OK
- 10.3934/jcd.2015005 is OK
- 10.1038/s41467-018-07210-0 is OK
- 10.1137/18M1177846 is OK
- 10.48550/arXiv.1710.04340 is OK
- 10.1017/jfm.2021.271 is OK
- 10.1016/j.ifacol.2016.10.250 is OK
- 10.1109/TAC.2020.2978039 is OK
- 10.1007/978-3-030-35713-9 is OK
- 10.1088/2632-2153/abf0f5 is OK
- 10.1016/j.automatica.2019.05.016 is OK
- 10.1137/20M1325678 is OK
- 10.21105/joss.02104 is OK
- 10.1137/17M115414X is OK
- 10.1016/j.physd.2004.06.015 is OK
- 10.1137/19M1267246 is OK
- 10.1137/15M1013857 is OK
- 10.1088/2632-2153/ac3de0 is OK

MISSING DOIs

- None

INVALID DOIs

- None
editorialbot commented 7 months ago

:warning: Error preparing paper acceptance. The generated XML metadata file is invalid.

ID ref-paszke2019pytorch already defined
ID ref-kaptanoglu2022pysindy already defined
olexandr-konovalov commented 7 months ago

@pswpswpsw I am investigating this - can't see any obvious reason for this, looking at references.

olexandr-konovalov commented 7 months ago

@pswpswpsw aaah, now it makes sense - look at https://github.com/dynamicslab/pykoopman/blob/master/docs/JOSS/paper.bib, it has duplicate labels paszke2019pytorch and kaptanoglu2022pysindy. Please fix!

pswpswpsw commented 7 months ago

@editorialbot check references

pswpswpsw commented 7 months ago

@editorialbot generate PDF

pswpswpsw commented 7 months ago

@olexandr-konovalov sorry. I just fixed this..

editorialbot commented 7 months ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1017/S0022112010001217 is OK
- 10.1137/1.9781611974508 is OK
- 10.5281/zenodo.3828935 is OK
- 10.21105/joss.03994 is OK
- 10.1146/annurev-fluid-030121-015835 is OK
- 10.1016/j.arcontrol.2009.12.001 is OK
- 10.1007/978-0-387-40065-5 is OK
- 10.21105/joss.00530 is OK
- 10.1017/9781108380690 is OK
- 10.1137/21M1401243 is OK
- 10.1063/1.4772195 is OK
- 10.1007/s00332-017-9437-7 is OK
- 10.1146/annurev-fluid-011212-140652 is OK
- 10.1063/1.4993854 is OK
- 10.1007/BF02532251 is OK
- 10.1038/s41467-017-00030-8 is OK
- 10.1007/978-3-662-04323-3_15 is OK
- 10.1017/S0022112009992059 is OK
- 10.1002/9781118535561 is OK
- 10.48550/arXiv.1710.09668 is OK
- 10.1137/18M1225409 is OK
- 10.1063/1.5011399 is OK
- 10.1038/s41467-017-02388-1 is OK
- 10.1098/rspa.2017.0844 is OK
- 10.1103/PhysRevLett.120.024102 is OK
- 10.1137/19M1274067 is OK
- 10.1016/j.jcp.2018.10.045 is OK
- 10.1073/pnas.1906995116 is OK
- 10.1126/science.aaw4741 is OK
- 10.1007/s00466-019-01711-5 is OK
- 10.1137/130932715 is OK
- 10.1016/j.cma.2016.03.025 is OK
- 10.1016/j.physd.2020.132401 is OK
- 10.1073/pnas.1118984109 is OK
- 10.1073/pnas.1620045114 is OK
- 10.1073/pnas.0609476104 is OK
- 10.1126/science.1165893 is OK
- 10.1038/ncomms9133 is OK
- 10.1073/pnas.1517384113 is OK
- 10.1007/s00332-015-9258-5 is OK
- 10.1126/sciadv.1602614 is OK
- 10.3934/jcd.2015005 is OK
- 10.1038/s41467-018-07210-0 is OK
- 10.1137/18M1177846 is OK
- 10.48550/arXiv.1710.04340 is OK
- 10.1017/jfm.2021.271 is OK
- 10.1016/j.ifacol.2016.10.250 is OK
- 10.1109/TAC.2020.2978039 is OK
- 10.1007/978-3-030-35713-9 is OK
- 10.1088/2632-2153/abf0f5 is OK
- 10.1016/j.automatica.2019.05.016 is OK
- 10.1137/20M1325678 is OK
- 10.21105/joss.02104 is OK
- 10.1137/17M115414X is OK
- 10.1016/j.physd.2004.06.015 is OK
- 10.1137/19M1267246 is OK
- 10.1137/15M1013857 is OK
- 10.1088/2632-2153/ac3de0 is OK

MISSING DOIs

- None

INVALID DOIs

- None
editorialbot commented 7 months ago

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

olexandr-konovalov commented 7 months ago

@editorialbot recommend-accept

editorialbot commented 7 months ago
Attempting dry run of processing paper acceptance...
editorialbot commented 7 months ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1017/S0022112010001217 is OK
- 10.1137/1.9781611974508 is OK
- 10.5281/zenodo.3828935 is OK
- 10.21105/joss.03994 is OK
- 10.1146/annurev-fluid-030121-015835 is OK
- 10.1016/j.arcontrol.2009.12.001 is OK
- 10.1007/978-0-387-40065-5 is OK
- 10.21105/joss.00530 is OK
- 10.1017/9781108380690 is OK
- 10.1137/21M1401243 is OK
- 10.1063/1.4772195 is OK
- 10.1007/s00332-017-9437-7 is OK
- 10.1146/annurev-fluid-011212-140652 is OK
- 10.1063/1.4993854 is OK
- 10.1007/BF02532251 is OK
- 10.1038/s41467-017-00030-8 is OK
- 10.1007/978-3-662-04323-3_15 is OK
- 10.1017/S0022112009992059 is OK
- 10.1002/9781118535561 is OK
- 10.48550/arXiv.1710.09668 is OK
- 10.1137/18M1225409 is OK
- 10.1063/1.5011399 is OK
- 10.1038/s41467-017-02388-1 is OK
- 10.1098/rspa.2017.0844 is OK
- 10.1103/PhysRevLett.120.024102 is OK
- 10.1137/19M1274067 is OK
- 10.1016/j.jcp.2018.10.045 is OK
- 10.1073/pnas.1906995116 is OK
- 10.1126/science.aaw4741 is OK
- 10.1007/s00466-019-01711-5 is OK
- 10.1137/130932715 is OK
- 10.1016/j.cma.2016.03.025 is OK
- 10.1016/j.physd.2020.132401 is OK
- 10.1073/pnas.1118984109 is OK
- 10.1073/pnas.1620045114 is OK
- 10.1073/pnas.0609476104 is OK
- 10.1126/science.1165893 is OK
- 10.1038/ncomms9133 is OK
- 10.1073/pnas.1517384113 is OK
- 10.1007/s00332-015-9258-5 is OK
- 10.1126/sciadv.1602614 is OK
- 10.3934/jcd.2015005 is OK
- 10.1038/s41467-018-07210-0 is OK
- 10.1137/18M1177846 is OK
- 10.48550/arXiv.1710.04340 is OK
- 10.1017/jfm.2021.271 is OK
- 10.1016/j.ifacol.2016.10.250 is OK
- 10.1109/TAC.2020.2978039 is OK
- 10.1007/978-3-030-35713-9 is OK
- 10.1088/2632-2153/abf0f5 is OK
- 10.1016/j.automatica.2019.05.016 is OK
- 10.1137/20M1325678 is OK
- 10.21105/joss.02104 is OK
- 10.1137/17M115414X is OK
- 10.1016/j.physd.2004.06.015 is OK
- 10.1137/19M1267246 is OK
- 10.1137/15M1013857 is OK
- 10.1088/2632-2153/ac3de0 is OK

MISSING DOIs

- None

INVALID DOIs

- None
editorialbot commented 7 months ago

:wave: @openjournals/csism-eics, this paper is ready to be accepted and published.

Check final proof :point_right::page_facing_up: Download article

If the paper PDF and the deposit XML files look good in https://github.com/openjournals/joss-papers/pull/5031, then you can now move forward with accepting the submission by compiling again with the command @editorialbot accept

danielskatz commented 7 months ago

Hi - I'm the track editor and will move this forward, but I'm currently on vacation, so please give me a few days (until Sunday or Monday likely)

danielskatz commented 7 months ago

👋 @pswpswpsw - please add countries to the affiliations in the paper. In addition, I've suggested some small changes in https://github.com/dynamicslab/pykoopman/pull/46 Please let me know when these two items are done (or if you disagree with any of them), and we can proceed.

pswpswpsw commented 7 months ago

@danielskatz Just finished. Both of them are approved and done!

pswpswpsw commented 7 months ago

@editorialbot generate PDF

editorialbot commented 7 months ago

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

danielskatz commented 7 months ago

@editorialbot recommend-accept

This will probably pause until tomorrow...

editorialbot commented 7 months ago
Attempting dry run of processing paper acceptance...
editorialbot commented 7 months ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1017/S0022112010001217 is OK
- 10.1137/1.9781611974508 is OK
- 10.5281/zenodo.3828935 is OK
- 10.21105/joss.03994 is OK
- 10.1146/annurev-fluid-030121-015835 is OK
- 10.1016/j.arcontrol.2009.12.001 is OK
- 10.1007/978-0-387-40065-5 is OK
- 10.21105/joss.00530 is OK
- 10.1017/9781108380690 is OK
- 10.1137/21M1401243 is OK
- 10.1063/1.4772195 is OK
- 10.1007/s00332-017-9437-7 is OK
- 10.1146/annurev-fluid-011212-140652 is OK
- 10.1063/1.4993854 is OK
- 10.1007/BF02532251 is OK
- 10.1038/s41467-017-00030-8 is OK
- 10.1007/978-3-662-04323-3_15 is OK
- 10.1017/S0022112009992059 is OK
- 10.1002/9781118535561 is OK
- 10.48550/arXiv.1710.09668 is OK
- 10.1137/18M1225409 is OK
- 10.1063/1.5011399 is OK
- 10.1038/s41467-017-02388-1 is OK
- 10.1098/rspa.2017.0844 is OK
- 10.1103/PhysRevLett.120.024102 is OK
- 10.1137/19M1274067 is OK
- 10.1016/j.jcp.2018.10.045 is OK
- 10.1073/pnas.1906995116 is OK
- 10.1126/science.aaw4741 is OK
- 10.1007/s00466-019-01711-5 is OK
- 10.1137/130932715 is OK
- 10.1016/j.cma.2016.03.025 is OK
- 10.1016/j.physd.2020.132401 is OK
- 10.1073/pnas.1118984109 is OK
- 10.1073/pnas.1620045114 is OK
- 10.1073/pnas.0609476104 is OK
- 10.1126/science.1165893 is OK
- 10.1038/ncomms9133 is OK
- 10.1073/pnas.1517384113 is OK
- 10.1007/s00332-015-9258-5 is OK
- 10.1126/sciadv.1602614 is OK
- 10.3934/jcd.2015005 is OK
- 10.1038/s41467-018-07210-0 is OK
- 10.1137/18M1177846 is OK
- 10.48550/arXiv.1710.04340 is OK
- 10.1017/jfm.2021.271 is OK
- 10.1016/j.ifacol.2016.10.250 is OK
- 10.1109/TAC.2020.2978039 is OK
- 10.1007/978-3-030-35713-9 is OK
- 10.1088/2632-2153/abf0f5 is OK
- 10.1016/j.automatica.2019.05.016 is OK
- 10.1137/20M1325678 is OK
- 10.21105/joss.02104 is OK
- 10.1137/17M115414X is OK
- 10.1016/j.physd.2004.06.015 is OK
- 10.1137/19M1267246 is OK
- 10.1137/15M1013857 is OK
- 10.1088/2632-2153/ac3de0 is OK

MISSING DOIs

- None

INVALID DOIs

- None
editorialbot commented 7 months ago

:wave: @openjournals/csism-eics, this paper is ready to be accepted and published.

Check final proof :point_right::page_facing_up: Download article

If the paper PDF and the deposit XML files look good in https://github.com/openjournals/joss-papers/pull/5047, then you can now move forward with accepting the submission by compiling again with the command @editorialbot accept

danielskatz commented 7 months ago

@editorialbot accept

editorialbot commented 7 months ago
Doing it live! Attempting automated processing of paper acceptance...