[REVIEW]: DMStag: Staggered, Structured Grids for PETSc

editorialbot commented 2 years ago

Submitting author: !--author-handle-->@psanan@kyleniemeyer<!--end-editor-- Reviewers: @elauksap, @mbarzegary Archive: 10.5281/zenodo.7315282

Status

Status badge code:

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

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

@kris-rowe & @elauksap & @mbarzegary, 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 @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 ✨

Checklists

📝 Checklist for @mbarzegary

📝 Checklist for @elauksap

editorialbot commented 2 years ago

Hello humans, I'm @editorialbot, a robot that can help you with some common editorial tasks.

For a list of things I can do to help you, just type:

@editorialbot commands

For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:

@editorialbot generate pdf

editorialbot commented 2 years ago

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

OK DOIs

- 10.1016/j.jcp.2011.09.007 is OK
- 10.1063/1.1761178 is OK

MISSING DOIs

- None

INVALID DOIs

- https://doi.org/10.1016/j.parco.2021.102831 is INVALID because of 'https://doi.org/' prefix

editorialbot commented 2 years ago

Software report:

github.com/AlDanial/cloc v 1.88  T=6.63 s (720.9 files/s, 202504.7 lines/s)
---------------------------------------------------------------------------------------
Language                             files          blank        comment           code
---------------------------------------------------------------------------------------
C                                     2247         123757         172349         621286
C/C++ Header                           453          10557          13171          66169
Python                                 433           7136           8664          49240
TeX                                      4           2812            186          33860
Perl                                    21           3107           1333          29711
C++                                     93           5110           6529          26841
Cython                                  83           4104           1302          21992
reStructuredText                       137           9437           8980          19972
Fortran 90                             182           3919           7157          16693
CUDA                                    29           1675           1482          13826
XML                                     14            389              3          13073
make                                   855           2219            488           9488
Bourne Shell                            23           1149            671           5190
HTML                                     5            205              6           3130
MATLAB                                  45            263            762           2825
JavaScript                              17            584            329           2303
YAML                                    11            117             72           1248
C Shell                                 66             72            154            614
CSS                                      5              3              0            497
SWIG                                     2             75             71            467
XSLT                                     1             30             28            460
GLSL                                    12             70             85            328
Markdown                                 6            106              0            300
Objective-C                              8            112            159            260
Bourne Again Shell                       4             34             61            173
Windows Module Definition                5              7              0            163
diff                                     7              0              0            153
Lisp                                     1              0              1            104
Jupyter Notebook                         1              0            493             55
Swift                                    2             25             28             40
DOS Batch                                1              8              1             27
CMake                                    1              5             18             20
INI                                      1              2              0             18
Java                                     1              0              0             13
TNSDL                                    1              0              0              4
JSON                                     1              0              0              3
---------------------------------------------------------------------------------------
SUM:                                  4778         177089         224583         940546
---------------------------------------------------------------------------------------

gitinspector failed to run statistical information for the repository

kyleniemeyer commented 2 years ago

👋 @psanan @kris-rowe, @elauksap, @mbarzegary

Each reviewer will have their own checklist, created with the command @editorialbot generate my checklist

editorialbot commented 2 years ago

Wordcount for paper.md is 973

editorialbot commented 2 years ago

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

mbarzegary commented 2 years ago

Review checklist for @mbarzegary

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 https://gitlab.com/petsc/petsc?
[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 (@psanan) 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, who the target audience is, and its relation to other work?
[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?

pcafrica commented 2 years ago

Review checklist for @elauksap

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 https://gitlab.com/petsc/petsc?
[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 (@psanan) 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, who the target audience is, and its relation to other work?
[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?

psanan commented 2 years ago

@editorialbot generate pdf

editorialbot commented 2 years ago

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

pcafrica commented 2 years ago

Hi @kyleniemeyer and @psanan!

I guess I completed my review of DMStag.

I was able to compile the software and to run the testsuite and the tutorials succesfully.

I marked all items in the checklist, including the Performance claims which do not apply in this case.

Being part of a large software (PETSc) the documentation of DMStag inherits parts such as installation instructions, community guidelines, etc. But I cannot think otherwise!

I would definitely recommend DMStag to be accepted for publication.

Best, Pasquale C. Africa

psanan commented 2 years ago

@elauksap Great to hear and thank you so much for your time and attention in reviewing our work!

kyleniemeyer commented 2 years ago

Thanks @elauksap!

mbarzegary commented 2 years ago

@editorialbot check references

editorialbot commented 2 years ago

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

OK DOIs

- 10.1016/j.jcp.2011.09.007 is OK
- 10.1016/j.parco.2021.102831 is OK
- 10.1063/1.1761178 is OK

MISSING DOIs

- None

INVALID DOIs

- None

mbarzegary commented 2 years ago

Hi @kyleniemeyer and @psanan,

I guess generally it's better to open an issue on the software repo for review comments, but since the submission is part of PETSc, I put the comments here instead of opening an issue on the PETSc repo in GitLab.

The software build and testing processes are straightforward as expected (I say “as expected” because it's part of PETSc), and the test cases and tutorials of DMStag build and run quite well. They also get installed using make install, showing that DMStag is fully integrated into PETSc.

The paper is written well. However, I have some suggestions to improve the quality of the submission:

Figures are unnecessarily too big on the paper (each is almost one page). I think reducing the size makes the paper look nicer.
It's not clear how the code is a standalone software and should be distinguished from the underlying PETSc code. I mean, you may add some explanation similar to what you described in the pre-review issue, but in a more straightforward language for non-specialists.
In line with the previous comment, this sentence doesn’t make sense: "with DMStag one can write code which will run on any number of MPI ranks". Is this because of PETSc or DMStag? It needs clarification.
I think more description should be added to the figures, like what we are looking at, what the size of the domain is, and this kind of things you expect to see in a scientific paper.
What should be set for runtime parameters of the examples to reproduce the figures? Is it enough to run them with default values? I think it is not, so it should be mentioned in the examples section.
I think adding a simple getting started guide can help a lot, like a short one for people with no prior knowledge of PETSc willing to try DMStag examples (people may find this paper later by looking for staggered grid simulations). It’s not meant to replace PETSc installation guide, but it can provide a short instruction for a minimal PETSc build suitable for running DMStag examples as well as the steps needed to run them (especially for the ones mentioned in the paper). I think it's also nice to mention the solvers and libraries needed to run the examples, such as mumps and umfpack.

kyleniemeyer commented 2 years ago

Thanks @mbarzegary for your feedback! This submission sort of dropped off my radar screen somehow.

@psanan, can you take a look and address the above comments?

psanan commented 2 years ago

Sorry for the delay - I will get to it ASAP.

kyleniemeyer commented 2 years ago

Hi @psanan, just wanted to check on your progress.

psanan commented 2 years ago

Hi @kyleniemeyer , apologies for the delay - as you can tell, I'm struggling to find time to wrap this up (new job). I'm hoping that the paper will be updated soon with with most of what @mbarzegary requested.

psanan commented 2 years ago

@editorialbot generate pdf

editorialbot commented 2 years ago

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

psanan commented 2 years ago

Thanks, @mbarzegary, for the review and apologies for the huge delay! The draft has just been updated and I'll respond to each comment below.

(1) Figures are unnecessarily too big on the paper (each is almost one page). I think reducing the size makes the paper look nicer.

Figures have been reduced to 60% width.

(2) It's not clear how the code is a standalone software and should be distinguished from the underlying PETSc code. I mean, you may add some explanation similar to what you described in the pre-review issue, but in a more straightforward language for non-specialists.

A note at the end of the summary has been added with this information.

(3) In line with the previous comment, this sentence doesn’t make sense: "with DMStag one can write code which will run on any number of MPI ranks". Is this because of PETSc or DMStag? It needs clarification.

This is enabled by the setting that PETSc provides, but of course DMStag has a lot of code written to support this (see e.g. stag3d.c which includes a lot of logic to build the required local to global maps). This language in the paper is aimed at users who are familiar with staggered grid but perhaps haven't used PETSc before, and might not be aware of this benefit. Clarification has been added to point out that this is true of all PETSc DM implementations.

(4) I think more description should be added to the figures, like what we are looking at, what the size of the domain is, and this kind of things you expect to see in a scientific paper.

Domain sizes and some short other details have been added to the paper.

(5) What should be set for runtime parameters of the examples to reproduce the figures? Is it enough to run them with default values? I think it is not, so it should be mentioned in the examples section.

Command line arguments to generate the images, and some short notes on what is loaded into Paraview, have been added.

(6) I think adding a simple getting started guide can help a lot, like a short one for people with no prior knowledge of PETSc willing to try DMStag examples (people may find this paper later by looking for staggered grid simulations). It’s not meant to replace PETSc installation guide, but it can provide a short instruction for a minimal PETSc build suitable for running DMStag examples as well as the steps needed to run them (especially for the ones mentioned in the paper). I think it's also nice to mention the solvers and libraries needed to run the examples, such as mumps and umfpack.

As it turns out, the two examples shown here don't require those direct solver libraries! The wave propagation example uses explicit timestepping, and the Stokes example uses an approximate block factorization (ABF) solver using PCFieldSplit with a nested MG solver on the velocity-velocity block, which seems to do fine with a coarse solve using the basic direct solver built into PETSc.

Hopefully the general PETSc installation instructions already give enough info for a minimal build, and that the explicit command line instructions for the examples should now be enough to run the specific examples here, without making this paper too long.

kyleniemeyer commented 1 year ago

@editorialbot generate pdf

kyleniemeyer commented 1 year ago

@editorialbot generate pdf

editorialbot commented 1 year ago

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

kyleniemeyer commented 1 year ago

Hi @psanan, there are just a few things with the references in the paper that need to be corrected:

Balay et al.: remove redundant URL field
Mills et al.: remove the "https://doi.org/" from the DOI field
Pusok et al.: add the DOI
Virieux 1986: add DOI (if available_

rtmills commented 1 year ago

Hi @kyleniemeyer. We have made the changes to the references that you have asked for and I have pushed this to our Gitlab repo.

psanan commented 1 year ago

@editorialbot generate pdf

editorialbot commented 1 year ago

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

kyleniemeyer commented 1 year ago

@psanan looks good! At this point please archive your software repo (e.g., using Zenodo) and let me know the DOI

psanan commented 1 year ago

Hi @kyleniemeyer - great! The version of PETSc used for this review has been uploaded to Zenodo. The DOI is

10.5281/zenodo.7315282

kyleniemeyer commented 1 year ago

@editorialbot set 10.5281/zenodo.7315282 as archive

editorialbot commented 1 year ago

Done! Archive is now 10.5281/zenodo.7315282

kyleniemeyer commented 1 year ago

@editorialbot recommend-accept

editorialbot commented 1 year ago

Attempting dry run of processing paper acceptance...

editorialbot commented 1 year ago

:wave: @openjournals/pe-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/3722, then you can now move forward with accepting the submission by compiling again with the command @editorialbot accept

editorialbot commented 1 year ago

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

OK DOIs

- 10.1190/1.1442147 is OK
- 10.1016/j.jcp.2011.09.007 is OK
- 10.1016/j.parco.2021.102831 is OK
- 10.1063/1.1761178 is OK
- 10.1093/gji/ggac309 is OK

MISSING DOIs

- None

INVALID DOIs

- None

kyleniemeyer commented 1 year ago

@editorialbot accept

editorialbot commented 1 year ago

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

editorialbot commented 1 year ago

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

editorialbot commented 1 year 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/3723
Wait a couple of minutes, then verify that the paper DOI resolves https://doi.org/10.21105/joss.04531
If everything looks good, then close this review issue.
Party like you just published a paper! 🎉🌈🦄💃👻🤘

Any issues? Notify your editorial technical team...

kyleniemeyer commented 1 year ago

@editorialbot remove @kris-rowe as reviewer

editorialbot commented 1 year ago

@kris-rowe removed from the reviewers list!

kyleniemeyer commented 1 year ago

@editorialbot reaccept

editorialbot commented 1 year ago

Rebuilding paper!

editorialbot commented 1 year ago

🌈 Paper updated!

New PDF and metadata files :point_right: https://github.com/openjournals/joss-papers/pull/3724

kyleniemeyer commented 1 year ago

Congratulations @psanan on your article's publication in JOSS!

Many thanks to @elauksap and @mbarzegary for reviewing this submission.

editorialbot commented 1 year 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.04531/status.svg)](https://doi.org/10.21105/joss.04531)

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

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

This is how it will look in your documentation:

We need your help!

The 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

psanan commented 1 year ago

Awesome! Thanks so much for all your attention, @kyleniemeyer, @mbarzegary , and @elauksap!

openjournals / joss-reviews