All reviews of published articles are made public. This includes manuscript files, peer review comments, author rebuttals and revised materials. Note: This was optional for articles submitted before 13 February 2023.
Peer reviewers are encouraged (but not required) to provide their names to the authors when submitting their peer review. If they agree to provide their name, then their personal profile page will reflect a public acknowledgment that they performed a review (even if the article is rejected). If the article is accepted, then reviewers who provided their name will be associated with the article itself.
One of the referees spotted a few grammatical errors and asked for a fact check regarding a CPU. Please incorporate these changes in the final TeX that you send to PeerJ.
No comments.
No comments.
No comments.
I am happy with the changes made to the revised manuscript and recommend publication.
Everything addressed.
Everything addressed.
Everything addressed.
All my comments seem to have been addressed.
In the changes I spotted the following problems that the authors might want to change for the final version:
+ line 394: "...refinement, and in inner loop overall grids
at..." should be "...refinement, and an inner loop over all grids
at..." (in->an, overall -> over all)
+ line 396: "... loop construct one thread..." Should be two
sentences: "...loop construct. One thread..."
+ line 399-400: "In addition there is a grids are limited to a maximum
of 32 cells in each dimension, otherwise they are bisected until
this condition is met." Sentence does not make sense. Maybe there
is a word missing.
One additional note:
+ line 407-408: Are you sure that is the CPU model and has 40 cores? I
could only find references to E5-2679v3 with 12 cores. But I am not a CPU expert. so do not bother.
I have a few comments in addition to the those of the referees.
- At the moment the title implies the paper is simply a description of the software, so it needs changing to something more descriptive. Perhaps something along the lines of "The Clawpack Software: Building an Open Source Ecosystem for Solving Nonlinear PDEs". I would be inclined not to put the version number in the tile.
- Page 1, line -10 of section 1.1: why are both [34], with its URL, and the explicit URL given?
- Page 1, line -3 of section 1.1: "with many of the changes from 4.3 to 4.6" appears to be part of an incomplete sentence, or at least one in need of rewriting.
- Page 5, line 4 of sec 2.2: Git submodules are new to me. Can you add a brief explanation of what they are?
- Page 6, line 8 of sec 2.3: "ensuring that past versions of the software remain available on a stable and citable platform". How is that done? I'm not sure if you say so later.
As regards the comments of Referee 2, you do not need to worry about reformatting issues, as PeerJ production takes care of these.
This article explains the main features and changes to the Clawpack 5.x software and its development practices and tools.
It is generally well written and reads well: good background, relevant citing, good structure etc.
This article is well within the scope of the journal, although I found it hard to find detailed information regarding what exactly is the scope of the journal, other than "articles across the whole of Computer Science".
Note that this is a software paper, describing features of a scientific software library. I view such papers as important contributions to the scientific literature and thus endorse its publication in PeerJ Computer Science.
Although this article is generally well written, I would suggest a number of improvements/clarifications:
1. On p. 2: What is q and what is f? And how does f (typically) depend on q (one or two examples). This is clear for most readers but please add one or two sentences to give a gentle introduction to non-specialists.
2. Some of the packages, e.g. PyCLAW on p. 3 and AMRCLAW on p. 4 are mentioned before it is made clear on p. 4-5 that these are separate packages in the collection that make up CLAWPACK. Please clarify earlier.
3. A diagram could be added to explain the relationships between the packages listed on p. 4-5.
4. Add a comma on line 163 (following "repositories") for increased readability.
5. The documentation repository is hosted on clawpack.github.com whereas the rest is hosted on github.com/clawpack/. This looks confusing to me.
6. Line 198: Should it be "Travis CI", not only "Travis"? Or is Travis the team member responsible for running the regression tests?
7. Top paragraph on p. 10: Somewhat confusing, seems to contrast AMR with MPI ("... on the other hand does not include AMR but uses MPI..."). Please reword.
8. Figures 2 and 3 are small and very difficult to read.
9. Remove FIXME/marginnote on p. 14.
1 Intro & Backround, Literature references
==========================================
The introduction is missing a list of related efforts/software
packages that are comparable to (parts of) ClawPack. This should be
added.
Furthermore a section should be added that lists all scientific
software that is used/needed by ClawPack together with
references. Currently, only PETSc and matplotlib are cited. According
to the website at least NumPy is needed/recommend and according to the
source matlab seems to be also used if it is found. These two should
be cited.
2 Structure conforms to Peerj Standard
======================================
On the cover page the corresponding author is not marked. He also
needs to be accompanied with his full address and email. Some
affiliations are missing their locations and countries. Please see
[Basic Manuscript Organization] for more detail.
Further more the format of the cover page is wrong.E.g. according to
the [example cover page] the affilliations should be listed directly
under the list of authors and not in the footer. But it seems like
this will be fixed by the publishers. No need for the author to change
this.
In contrast to the [PeerJ standard] the Acknowledgement section also
acknowledges funders, too. This information should be moved to a new
section "Funding statement".
The in-text citation style is not in the correct format (e.g on the
cover pages [1] is used instead of Le Veque, 2002). But it seems like
this will be fixed by publisher. No need for the authors to do it.
Consequently, the references sections uses the wrong format,
too. Please see the [reference section in the standard]. While it
seems like the correct order (Authors with initials, year, title,
journal) will be adjusted by the publishers, the author are using
abbreviated journal titles. This needs to be changed to full journal
titles as requested by the standard.
The authors should double check all the links in the Refernce section.
At least the one in reference 6 is broken.
A lot of the references lack dois. The authors should double check
whether the publicaction do really not have any DOI associated with
it. At least reference 3 (authored by at least one of the authors
themselves) does have a DOI (0.1137/S106482750139738X). DOIs should be
added wherever they are available.
[Basic Manuscript Organization]
https://peerj.com/about/author-instructions/#basic-manuscript-organization
[example cover page]
https://peerj.com/about/author-instructions/#manuscript-checks-modal
[PeerJ standard]
https://peerj.com/about/author-instructions/#standard-sections
[reference section in the standard]
https://peerj.com/about/author-instructions/#reference-section
3 Figures
=========
The text in the pictures of Figure 5 cannot be read on a laptop
screen. Maybe increasing the size of the text or the pictures
themselves would fix this. Some of the pictures seem to be or use
third party pictures. The author should checkshether their license
allows the distribution under CC-BY. Some of the pictures (e.g. the
maps in Figure 3 and 5 might need a copyright attribute added. See
[Figure/Table referencing]
The referencing style for figures should be used consistently. Figure 1
is referenced as "Figure 1" while the rest is referenced as "Fig. 2",
etc.
[Figure/Table referencing]
https://peerj.com/about/author-instructions/cs#figure-referencing
The paper describes the changes of the scientific software ClawPack
between version 4.x and 5.3, its development history, and the newly
adopted development model. It is a nice software release paper that
are currently becoming more and more popular. I haved checked with the
editors and they are accepting such papers. Therefore I will neglect
reviewing guidelines that cannot be applied to release papers.
Just a few minor comments:
lines 27--28 (Introduction): I think it would be nice to list one/some of the major
changes that will make upgrading to the new user worthwhile or make
ClawPack more unique. (Research question)
lines 563--570 (Conclusion): Similar to above, I am missing a short sentence why
upgrading is worthwhile for a user. The current conclusion only takes
into account the view of developers.
I don't think this section applies to software release papers.
lines 49--50:
Reference [34] contains the URL. Therefore the last sentence (lines
49-50) is superluous.
lines 216--223:
In the abstract it says that clawpack has been developed as open
source for 20 years. Here it reads like the software has been made
open source recently as you expect it to increase the developer
comunity. Please clarify!
line 227:
Citation of PETSc reference missing
line 354:
"GEOCLAW. where" has a superfluos dot,
line 358:
" ..., where some operation ..."
Is it really some (might be different per thread) or the same
operation?
lines 357--362:
"The main ... patch at a time"
Application of parallel_for is not clear to me. First it seems like
parallelization will be over the patches. Then you say that each
iteration corresponds to a grid level (shouldn't that be a
patch?). Afterwards you say you assign one patch to each thread. It
might also be that my understanding of a grid is different from the
author's. Please clarify, currently the parallelization strategy is
not clear to me.
line 368:
"Figure 1" -> "Fig. 1" for consistency.
lines 370--371:
40 cores or 20 cores/40 threads? Which exact processor model did you
use?
line 377:
"Note that there are only 2 level 1" -> ""Note that there are only two
level 1"
line 376:
"The efficiency is above 80% until 24 cores, then drops..."
I would have expected it drop for >20 threads as two threads share one
core. Might the preservation of efficiency be due to the ordering of
the patch by size? Do you have an explanation?
lines 377--380:
I don't understand these sentences about level n grids. Does two level
1 grids mean that 2 cells on level 0 got refined? Maybe the
meaning/definition of a leveln grid could/should be stated.
line 382:
Citation of PetSc missing.
Figure 3:
The text in the left two pictures is hardly readable on a laptop
screen. Maybe making the picture larger would help.
If these are 3rdparty pictures, please check whether they can be
distributed via CC-BY Licence. At least for the map a reference is
missing.
Figure 5:
Please double check whether you are allowed to distribute the pictures
under CC-BY License and whether they need a reference to be added.
lines 480-482:
Is the definition of patch here the same as in the description of the
OpenMP parallelization above (lines 357--362)? If this is the case
then the mesh-patch equivalence should be mentioned there.
Page 14:
There is an annotation in the original PDF.h
All text and materials provided via this peer-review history page are made available under a Creative Commons Attribution License, which permits unrestricted use, distribution, and reproduction in any medium, provided the original author and source are credited.