joss-reviews
joss-reviews copied to clipboard
openjournals
[REVIEW]: PERFORM: A Python package for developing reduced-order models for reacting fluid flows
Submitting author: @cwentland0 (Christopher Wentland) Repository: https://github.com/cwentland0/perform Branch with paper.md (empty if default branch): Version: v0.1 Editor: @kyleniemeyer Reviewers: @Himscipy, @kyleniemeyer Archive: Pending
: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/690515fb80b2e2c5d7635084159f2cae"><img src="https://joss.theoj.org/papers/690515fb80b2e2c5d7635084159f2cae/status.svg"></a>
Markdown: [](https://joss.theoj.org/papers/690515fb80b2e2c5d7635084159f2cae)
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
@clemaitre58 & @Himscipy, 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 @kyleniemeyer 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 @clemaitre58
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
- [ ] 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?
- [ ] Contribution and authorship: Has the submitting author (@cwentland0) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
- [ ] Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines
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: Does the paper have a section titled 'Statement of Need' that clearly states 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 @Himscipy
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 (@cwentland0) 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: Does the paper have a section titled 'Statement of Need' that clearly states 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?
Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @clemaitre58, @Himscipy 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:
- 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
For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:
@whedon generate pdf
Software report (experimental):
github.com/AlDanial/cloc v 1.88 T=0.17 s (468.0 files/s, 58524.4 lines/s)
-------------------------------------------------------------------------------
Language files blank comment code
-------------------------------------------------------------------------------
Python 49 1836 2497 3807
reStructuredText 23 529 696 571
Markdown 5 43 0 55
Bourne Shell 2 14 8 46
make 1 4 6 10
TeX 1 0 0 7
-------------------------------------------------------------------------------
SUM: 81 2426 3207 4496
-------------------------------------------------------------------------------
Statistical information for the repository '0b642e42e6e4e096221ded4d' was
gathered on 2021/06/28.
The following historical commit information, by author, was found:
Author Commits Insertions Deletions % of changes
Ashish S Nair 14 2181 996 6.75
chris 144 25301 18592 93.25
Below are the number of rows from each author that have survived and are still
intact in the current revision:
Author Rows Stability Age % in comments
chris 8140 32.2 1.1 11.20
PDF failed to compile for issue #3428 with the following error:
Can't find any papers to compile :-(
👋 Hello @cwentland0 @clemaitre58 @Himscipy we will perform the actual review in this issue. Thanks!
Attempting PDF compilation from custom branch paper. Reticulating splines etc...
:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:
:wave: @clemaitre58, please update us on how your review is going (this is an automated reminder).
:wave: @Himscipy, please update us on how your review is going (this is an automated reminder).
@kyleniemeyer, I am unable to check mark the actions in the reviewers checklist.
The reviewer already has a pending invite.
@himscipy please accept the invite by clicking this link: https://github.com/openjournals/joss-reviews/invitations
OK, the reviewer has been re-invited.
@himscipy please accept the invite by clicking this link: https://github.com/openjournals/joss-reviews/invitations
The reviewer already has a pending invite.
@himscipy please accept the invite by clicking this link: https://github.com/openjournals/joss-reviews/invitations
@kyleniemeyer A question, since I'm unfamiliar with software journal reviews in general, especially so when I first submitted this back in April. There have been some significant changes to the code in a couple branches that will soon be merged, including general expansions of capabilities and some class names which were explicitly mentioned in the paper. Should I update the paper to reflect these changes once they're merged (or at the very least, remove any specific references to class names, which was a naive mistake on my part)? I understand the reviewers may have already looked over the paper, but I figure these changes would be mostly superficial, and would not change the bulk of its content. I know there's usually some back and forth with editing, I just don't want to waste any of the reviewers' time.
@cwentland0 good question - this is always tough when software continues to evolve while under review. Minor edits to the paper are fine, especially if you summarize the changes in here for us.
Larger edits to the software, that change capabilities and would essentially require a re-review, should probably wait, so that the version mentioned with a JOSS article is actually the version reviewed.
Maybe "significant changes" was a little overblown on my part. To be more specific, the changes will add two more classes of ROM methods, which would amount to adding two more sub-bullet points under the current "ROMs" bullet point on page 3. Would you consider that a larger edit?
👋🏼 Hi @cwentland0 , Thank you for submitting the paper+ software to JOSS. The developed package will really a nice addition to the scientific community both for research and academic purpose. I am currently reviewing your code base, one thing which I caught my attention is that "testing" of any sort is missing in the code base. Tests plays a crucial role for any software package and is one of the essential components in the JOSS submissions acceptance criteria, therefore I would like to recommend that you add these testing procedures to the code base. Thank you.
@Himscipy thank you for taking the time to review the code. You're right, I haven't implemented anything in the way of rigorous unit or regression tests, I've only provided a few pre-set cases to ensure base functionality of the solver and few ROMs. I'll get to work on a better testing framework.
Hi @kyleniemeyer , sorry for the delayed response. Unfortunately I've been pulled in a couple different directions the past month and was only able to add unit tests for most fundamental classes in the code (in an unmerged branch so far). I have not implemented integration tests for the solver/ROM routines. I can hopefully dedicate some time to this in the coming weeks, I really appreciate your patience.
Hi all, just wanted to give an update. I have put together a fairly thorough testing framework of unit, integration, and regression tests which can be found in the tests/ directory. Instructions for manually running the tests have been included in the README. Additionally, I have added continuous integration via GitHub Actions, which automatically executes unit and integration tests and reports code coverage via the Coveralls API. Regression tests are run nightly and aren't included in the code coverage. Hopefully this covers the testing requirement.
Additionally, I have made minor changes to the paper to a) sanitize it of any specific mention of class names, since those might change and evolve over time, and b) add a citation for a recent paper that has used PERFORM. No other substantive changes were made to the paper. I had to make a Zenodo DOI for the repository to make the code citeable for collaborators, I think I recall that this can be substituted for the JOSS paper DOI in the event of acceptance?
Thank you for your patience!
Attempting PDF compilation from custom branch paper. Reticulating splines etc...