[REVIEW]: Coral: a parallel spectral solver for fluid dynamics and partial differential equations

whedon commented 3 years ago

Submitting author: @benmql (BENJAMIN MIQUEL) Repository: https://github.com/BenMql/coral Version: v1.1.12 Editor: @eloisabentivegna Reviewer: @robertsawko, @rhaas80 Archive: 10.5281/zenodo.5458888

: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/9cb3168716912f13021339e58117fdee"><img src="https://joss.theoj.org/papers/9cb3168716912f13021339e58117fdee/status.svg"></a>
Markdown: [![status](https://joss.theoj.org/papers/9cb3168716912f13021339e58117fdee/status.svg)](https://joss.theoj.org/papers/9cb3168716912f13021339e58117fdee)

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

@robertsawko & @rhaas80, 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 @eloisabentivegna 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 @robertsawko

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 (@benmql) 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: Do the authors clearly state 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?

Review checklist for @rhaas80

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 (@benmql) 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: Do the authors clearly state 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?

danielskatz commented 3 years ago

👋 @rhaas80 @robertsawko @BenMql - ping... see ☝️

rhaas80 commented 3 years ago

I just finished reading through the source. Looks good to me. Overall quite readable. For some files (most of the LP Coral file) there are haders:

 !============================================================================= 
 !                            C O R A L 
 !============================================================================= 
 ! 
 ! MODULE: LP_IMEX_timestepping
 !  
 !> @author 
 !> Ben Miquel, www.normalesup.org/~benmiquel, benjamin.miquel@tutanota.com 
 ! 
 ! DESCRIPTION 
 !> core routines and data structure implementing the time-marching schemes
 !> of Ascher-Ruuth-Spiteri 1997 (Applied Numerical Mathematics 25, pp151-167)
 !> [denoted below as ARS97].
 ! 
 !=============================================================================

but not quite for all. Since these are very useful to get a rough idea what may be expected in a file, having them for all files would be great (but purely optional).

rhaas80 commented 3 years ago

so once the two issues related to community contribution and quantitative tests are addressed I have no further objections.

BenMql commented 3 years ago

Thank you @rhaas80 and @robertsawko for the careful reviews. @danielskatz, I will be back in the office on Monday and will address the last suggestions promptly.

BenMql commented 3 years ago

:wave: all,

I believe I have addressed all the remaining issues, which I summarize here:

both @rhaas80 and @robertsawko mentioned the lack of quantitative tests (e.g. here). I have now gathered all the tests in a dedicated wiki page here. For each benchmark, users are provided with a python routine so that the computation of global quantities is straightforward, and these values can readily be compared to reference values given in the same wiki page.
both @rhaas80 and @robertsawko mentioned the lack of community guidelines (e.g. here. A CONTRIBUTING.md file has been added to fill this void and address this issue.
The third item missing in @robertsawko 's checklist is: "Performance: If there are any performance claims of the software, have they been confirmed? (If there are no claims, please check off this item.)" This is a bit of a grey area, but perhaps this item could be checked off as there is indeed no claim for Coral itself (although 2decomp&fft has been benchmarked by their authors). In the long term, a systematic performance study will be available on the wiki. For now, I do not have enough data points though.
@robertsawko mentioned the relevance of a custom sidebar (as the number of pages increases), which I have happily implemented.
@rhaas80 mentioned the lack of consistency regarding headers in the source file, as an optional comment. This is a good point. More generally, these headers are part of the documentation of the code written in Doxygen. This documentation is still work in progress but will hopefully cover all source files and subroutines (at which point the headers will be homogeneous).

Apologies for this long interruption during Summer. Again, I want to express my gratitude for your suggestions which are very helpful and result in improved sources/doc. Let me know if I have missed something.

danielskatz commented 3 years ago

@robertsawko & @rhaas80 - please take a look at @BenMql's changes, and see if you can check off any remaining items. If not, please state what additional work you think is needed

rhaas80 commented 3 years ago

contribution.md looks very good. While no automated tests are provided the manual tests now come with a textual description of the expected numerical output that lets users manually verify that their copy of CORAL works as expected.

I have no further comments that would delay publication.

danielskatz commented 3 years ago

👋 @robertsawko - How do things now look from your side?

robertsawko commented 3 years ago

Hello and thanks to @danielskatz for a ping on another email.

@BenMql thanks for addressing all of the comments. I think the wiki looks very clear now and examples are well described. I don't have any further comments. I've checked the remaining boxes.

danielskatz commented 3 years ago

Thanks @robertsawko

danielskatz commented 3 years ago

@BenMql - I'll proofread the paper shortly, then give you a few final steps (perhaps including suggested text changes) to do.

danielskatz commented 3 years ago

@whedon check references

whedon commented 3 years ago

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

OK DOIs

- 10.1017/jfm.2020.485 is OK
- 10.1103/physrevfluids.4.121501 is OK
- 10.1017/S0022112096002789 is OK
- 10.1103/PhysRevFluids.5.113702 is OK
- 10.1103/PhysRevE.70.056312 is OK
- 10.1103/PhysRevResearch.2.023068 is OK
- 10.1016/j.cpc.2019.107110 is OK
- 10.21105/joss.01071 is OK
- 10.1515/jnum-2012-0013 is OK
- 10.5334/jors.237 is OK
- 10.5334/jors.239 is OK

MISSING DOIs

- 10.1063/1.3058072 may be a valid DOI for title: Hydrodynamic and Hydromagnetic Stability

INVALID DOIs

- None

danielskatz commented 3 years ago

@whedon generate pdf

whedon commented 3 years ago

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

danielskatz commented 3 years ago

@BenMql - I've suggested some changes in https://github.com/BenMql/coral/pull/22 - please either merge these or let me know what you disagree with

BenMql commented 3 years ago

@danielskatz -- thank you for your edits, much appreciated. They have been merged.

danielskatz commented 3 years ago

@whedon generate pdf

whedon commented 3 years ago

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

danielskatz commented 3 years ago

@BenMql - I missed one issue, which will be fixed if you merge https://github.com/BenMql/coral/pull/23 (I hope) - Please merge this, and run @whedon generate pdf to check if it looks ok. If so, then please go ahead and

[ ] Make a tagged release of your software, and list the version tag of the archived version here.
[ ] Archive the reviewed software in Zenodo or a similar service (e.g., figshare, an institutional repository)
[ ] Check the archival deposit (e.g., in Zenodo) has the correct metadata. This includes the title (should match the paper title) and author list (make sure the list is correct and people who only made a small fix are not on it). You may also add the authors' ORCID.
[ ] Please list the DOI of the archived version here.

I can then move forward with accepting the submission.

BenMql commented 3 years ago

@whedon generate pdf

whedon commented 3 years ago

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

danielskatz commented 3 years ago

@BenMql - you might want to change the bib entry for your J Fluid Mech paper to have your first initial rather than first name for consistency, but it's not a big deal either way

BenMql commented 3 years ago

@whedon generate pdf

whedon commented 3 years ago

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

BenMql commented 3 years ago

@danielskatz That looks better. The version tag is v1.1.12 What do people customarily do: register the upload on Zenodo as software or publication? (Apologies for the stupid question, but after all there is a paper and a code in the archive.)

danielskatz commented 3 years ago

the repo (which can but doesn't have to include the paper) should be registered as software - some people use https://guides.github.com/activities/citable-code/ and then make a tagged release on github, which creates the zenodo deposit - if you do this, you will then need to change the zenodo metadata to match the paper title and authors manually

BenMql commented 3 years ago

@danielskatz -- the archived Coral v1.1.12 has been uploaded on Zenodo and has received the DOI 10.5281/zenodo.5458888

danielskatz commented 3 years ago

@whedon set v1.1.12 as version

whedon commented 3 years ago

OK. v1.1.12 is the version.

danielskatz commented 3 years ago

@whedon set 10.5281/zenodo.5458888 as archive

whedon commented 3 years ago

OK. 10.5281/zenodo.5458888 is the archive.

danielskatz commented 3 years ago

@whedon recommend-accept

whedon commented 3 years ago

Attempting dry run of processing paper acceptance...

whedon commented 3 years ago

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

OK DOIs

- 10.1017/jfm.2020.485 is OK
- 10.1103/physrevfluids.4.121501 is OK
- 10.1017/S0022112096002789 is OK
- 10.1103/PhysRevFluids.5.113702 is OK
- 10.1103/PhysRevE.70.056312 is OK
- 10.1103/PhysRevResearch.2.023068 is OK
- 10.1016/j.cpc.2019.107110 is OK
- 10.21105/joss.01071 is OK
- 10.1515/jnum-2012-0013 is OK
- 10.5334/jors.237 is OK
- 10.5334/jors.239 is OK

MISSING DOIs

- 10.1063/1.3058072 may be a valid DOI for title: Hydrodynamic and Hydromagnetic Stability

INVALID DOIs

- None

whedon commented 3 years ago

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

Check final proof :point_right: https://github.com/openjournals/joss-papers/pull/2569

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

@whedon accept deposit=true

danielskatz commented 3 years ago

@whedon accept deposit=true

whedon commented 3 years ago

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

whedon commented 3 years ago

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

whedon commented 3 years ago

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

Here's what you must now do:

Check final PDF and Crossref metadata that was deposited :point_right: https://github.com/openjournals/joss-papers/pull/2570
Wait a couple of minutes, then verify that the paper DOI resolves https://doi.org/10.21105/joss.02978
If everything looks good, then close this review issue.
Party like you just published a paper! 🎉🌈🦄💃👻🤘

Any issues? Notify your editorial technical team...

danielskatz commented 3 years ago

@BenMql - The DOI doesn't yet resolve for me - when it does, I will close this issue and we'll be done.

BenMql commented 3 years ago

Fantastic. Thank you for your assistance. The DOI doesn't resolve yet for me neither, but it is probably only a question of time.

danielskatz commented 3 years ago

Congratulations to @BenMql (BENJAMIN MIQUEL)!!

(The DOI is now working, so we're done)

And thanks to @eloisabentivegna for editing, and @robertsawko and @rhaas80 for reviewing! We couldn't do this without you!

whedon commented 3 years ago

:tada::tada::tada: Congratulations on your paper acceptance! :tada::tada::tada:

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

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

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

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.org/donate-to-joss

openjournals / joss-reviews