search

openjournals / joss-reviews

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

[REVIEW]: Mandyoc: A finite element code to simulate thermochemical convection in parallel #4070

Closed whedon closed 2 years ago

whedon commented 2 years ago

Submitting author: !--author-handle-->@victorsacek<!--end-author-handle-- (Victor Sacek) Repository: https://github.com/ggciag/mandyoc Branch with paper.md (empty if default branch): Version: v0.1.6 Editor: !--editor-->@jedbrown<!--end-editor-- Reviewers: @rbeucher, @psanan Archive: 10.5281/zenodo.6390220

: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

Status badge code:

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

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

@rbeucher, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:

  1. Make sure you're logged in to your GitHub account
  2. 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 @jedbrown 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 @rbeucher

✨ Important: Please do not use the Convert to issue functionality when working through this checklist, instead, please open any new issues associated with your review in the software repository associated with the submission. ✨

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Review checklist for @psanan

✨ Important: Please do not use the Convert to issue functionality when working through this checklist, instead, please open any new issues associated with your review in the software repository associated with the submission. ✨

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

whedon commented 2 years ago

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @rbeucher 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:

  1. Set yourself as 'Not watching' https://github.com/openjournals/joss-reviews:

watching

  1. You may also like to change your default settings for this watching repositories in your GitHub profile here: https://github.com/settings/notifications

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
whedon commented 2 years ago

Wordcount for paper.md is 871

whedon commented 2 years ago 
Software report (experimental):

github.com/AlDanial/cloc v 1.88  T=0.08 s (639.0 files/s, 154101.0 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
C++                             14           2302            350           5175
Python                           9            266            430            669
Markdown                         7            134              0            359
TeX                              2             34              0            342
C/C++ Header                     1             91             10            244
reStructuredText                 9            335            398            243
make                             2             20              6             76
YAML                             1              9             21             37
Bourne Shell                     2              4              0             10
CSS                              1              1              0              9
-------------------------------------------------------------------------------
SUM:                            48           3196           1215           7164
-------------------------------------------------------------------------------

Statistical information for the repository '21250cc553caf1aa2ba1642b' was
gathered on 2022/01/15.
The following historical commit information, by author, was found:

Author                     Commits    Insertions      Deletions    % of changes
Agustina                         4           109             67            1.28
Agustina Pesce                   1            17             10            0.20
Dave May                         1             2              0            0.01
Jamison Assunção                 3           716            700           10.26
Rafael Monteiro da S             3           391              4            2.86
Rafael Silva                     5           908             23            6.75
Victor Sacek                    69          9325           1524           78.64

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
Agustina                    536          491.7          0.0                9.33
Dave May                      2          100.0         18.1                0.00
Jamison Assunção            637           89.0          3.0               23.86
Rafael Monteiro da S       1094          279.8          8.8                4.94
Rafael Silva                  1            0.1          0.5                0.00
Victor Sacek               7267           77.9          3.7                3.95
whedon commented 2 years ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1051/0004-6361/201629272 is OK
- 10.1051/0004-6361/201322068 is OK

MISSING DOIs

- 10.1111/j.1365-246x.2012.05388.x may be a valid DOI for title: A comparison of numerical surface topography calculations in geodynamic modelling: an evaluation of the ‘sticky air’method
- 10.1016/j.pepi.2010.04.007 may be a valid DOI for title: A stabilization algorithm for geodynamic numerical simulations with a free surface
- 10.1007/978-1-4612-1986-6_8 may be a valid DOI for title: Efficient Management of Parallelism in Object Oriented Numerical Software Libraries
- 10.1016/j.epsl.2016.11.026 may be a valid DOI for title: Post-rift influence of small-scale convection on the landscape evolution at divergent continental margins
- 10.1016/j.jog.2021.101830 may be a valid DOI for title: Lateral flow of thick continental lithospheric mantle during tectonic quiescence
- 10.1029/97jb01353 may be a valid DOI for title: A comparison of methods for the modeling of thermochemical convection

INVALID DOIs

- None
whedon 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:

jedbrown commented 2 years ago

Hi @rbeucher! :wave: Welcome to JOSS and thanks for agreeing to review! The comments from @whedon above outline the review process, which takes place in this thread (possibly with issues filed in the Mandyoc repository). I'll be watching this thread if you have any questions.

The JOSS review is different from most other journals. Our goal is to work with the authors to help them meet our criteria instead of merely passing judgment on the submission. As such, the reviewers are encouraged to submit issues and pull requests on the software repository. When doing so, please mention this issue so that a link is created to this thread (and I can keep an eye on what is happening). Please also feel free to comment and ask questions on this thread. In my experience, it is better to post comments/questions/suggestions as you come across them instead of waiting until you've reviewed the entire package.

We aim for reviews to be completed within a month or so. Please let me know if you require some more time. We can also use Whedon (our bot) to set automatic reminders if you know you'll be away for a known period of time.

Please feel free to ping me (@jedbrown) if you have any questions/concerns.

jedbrown commented 2 years ago

@victorsacek Please check out those missing DOI reported above :point_up:

whedon commented 2 years ago

:wave: @rbeucher, please update us on how your review is going (this is an automated reminder).

rbeucher commented 2 years ago

I'm starting the review today. Sorry for the delay.

rbeucher commented 2 years ago

@victorsacek

Thanks for your submission. I have installed the code and I am currently running the examples. So far so good. I have opened a series of issues on the repository issue tracker with some comments and suggestions on how to make the submission better.

The code compiles and run as described.

It needs some clearer examples that cover the range of functionalities describe in the documentation/paper. The documentation is incomplete. It also needs simple tutorials to get the user started. I would strongly suggest some basic automated tests....

The paper needs some work both in terms of structure and content. I think the content needs to be aligned with the title. See my comments on the issue tracker.

jedbrown commented 2 years ago

@whedon add @psanan as reviewer

whedon commented 2 years ago

OK, @psanan is now a reviewer

jedbrown commented 2 years ago

Hi @psanan! :wave: Welcome to JOSS and thanks for agreeing to review! The comments from @whedon above outline the review process, which takes place in this thread (possibly with issues filed in the Mandyoc repository). I'll be watching this thread if you have any questions.

The JOSS review is different from most other journals. Our goal is to work with the authors to help them meet our criteria instead of merely passing judgment on the submission. As such, the reviewers are encouraged to submit issues and pull requests on the software repository. When doing so, please mention this issue so that a link is created to this thread (and I can keep an eye on what is happening). Please also feel free to comment and ask questions on this thread. In my experience, it is better to post comments/questions/suggestions as you come across them instead of waiting until you've reviewed the entire package.

We aim for reviews to be completed within a month or so. Please let me know if you require some more time. We can also use Whedon (our bot) to set automatic reminders if you know you'll be away for a known period of time.

Please feel free to ping me (@jedbrown) if you have any questions/concerns.

psanan commented 2 years ago

@whedon generate pdf

whedon 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

@victorsacek, thanks for the submission! @rbeucher has opened several issues on the Mandyoc tracker, which cover the majority of points I wanted to make, at least generally, so I will just add my own comments to those issues where appropriate, and open new ones (limited in scope) if one of those issues is resolved but I still think some further change is required for acceptance.

psanan commented 2 years ago

@whedon generate pdf

editorialbot commented 2 years ago

My name is now @editorialbot

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:

rafaelmds commented 2 years ago

@rbeucher @psanan

We published a new release of Mandyoc v.0.1.5 addressing most of the review issues.

We kept issues open for now, since we made comments on each of them.

rafaelmds 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

@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

@jedbrown @victorsacek I've checked off everything on my review list - looks ready from my perspective! Thanks again for the submission and the code - I particularly appreciate the attention paid to thorough documentation, including easy-to-follow, well-discussed, and reproducible examples covering both a benchmark case and a more realistic model.

rbeucher commented 2 years ago

@jedbrown @victorsacek I went through the changes and it looks ready to me. Thanks for doing the extra work!

jedbrown commented 2 years ago

Thanks for your diligent reviews @psanan @rbeucher.

@victorsacek Once https://github.com/ggciag/mandyoc/pull/83 is merged (or you've decided to revise otherwise to address these points), please tag a release (annotated tags preferred) and archive your release on Zenodo or similar, then report the DOI back here.

jedbrown 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.1111/j.1365-246X.2012.05388.x is OK
- 10.1016/j.pepi.2010.04.007 is OK
- 10.1007/978-1-4612-1986-6_8 is OK
- 10.1016/j.epsl.2016.11.026 is OK
- 10.1016/j.jog.2021.101830 is OK
- 10.1029/97JB01353 is OK

MISSING DOIs

- 10.11606/t.14.2021.tde-11052021-184220 may be a valid DOI for title: Influence of surface processes on post-rift faulting during divergent margins evolution
- 10.21105/joss.01136 may be a valid DOI for title: UWGeodynamics: A teaching and research tool for numerical geodynamic modelling
- 10.1016/j.pepi.2007.06.009 may be a valid DOI for title: Computational approaches to studying non-linear dynamics of the crust and mantle
- 10.31223/x5q333 may be a valid DOI for title: Evolution of rift systems and their fault networks in response to surface processes
- 10.1029/2019gc008884 may be a valid DOI for title: Morphotectonic Evolution of Passive Margins Undergoing Active Surface Processes: Large-Scale Experiments Using Numerical Models

INVALID DOIs

- http://doi.org/10.5281/zenodo.5131909 is INVALID because of 'https://doi.org/' prefix
- http://doi.org/10.6084/m9.figshare.4865333.v8 is INVALID because of 'https://doi.org/' prefix
- http://doi.org/10.1111/j.1365-246X.2012.05609.x is INVALID because of 'https://doi.org/' prefix
- http://doi.org/10.1093/gji/ggx195 is INVALID because of 'https://doi.org/' prefix
jedbrown commented 2 years ago

:point_up: The PR addresses these.

victorsacek commented 2 years ago

Dear @jedbrown, @rbeucher, and @psanan

Thank you very much for the detailed revision of our code and documentation. Zenodo archived the version v0.1.5.1 of the Mandyoc code: 10.5281/zenodo.6373350

jedbrown commented 2 years ago

@editorialbot set v0.1.5.1 as archive

editorialbot commented 2 years ago

Done! Archive is now v0.1.5.1

jedbrown commented 2 years ago

@victorsacek Please edit the metadata on your DOI so the authors match this paper.

jedbrown commented 2 years ago

@editorialbot set 10.5281/zenodo.6373350 as archive

editorialbot commented 2 years ago

Done! Archive is now 10.5281/zenodo.6373350

jedbrown commented 2 years ago

@editorialbot set v0.1.5.1 as version

editorialbot commented 2 years ago

Done! version is now v0.1.5.1

jedbrown commented 2 years ago

@editorialbot generate pdf

jedbrown commented 2 years ago

@editorialbot check references

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:

victorsacek commented 2 years ago

@victorsacek Please edit the metadata on your DOI so the authors match this paper.

Thank you @jedbrown. Now the metadata was corrected.

victorsacek commented 2 years ago

Dear @jedbrown

We observed that the tag for the last release is an invalid semver. We chose four numbers (v0.1.5.1) instead of three. The correct version must be v0.1.6. What is the best option to correct this mistake?

Thank you for your attention.

jedbrown commented 2 years ago

@victorsacek You can either tag v0.1.6 and update Zenodo or we can work with v0.1.5.1 (JOSS doesn't require that you use semver, strictly or otherwise). Your choice, just let us know which you choose.

victorsacek commented 2 years ago

Dear @jedbrown Thank you for your answer. We decided to update the tag to v0.1.6. Here is the new Zenodo DOI: 10.5281/zenodo.6390220

jedbrown commented 2 years ago

@editorialbot set v0.1.6 as version

editorialbot commented 2 years ago

Done! version is now v0.1.6

jedbrown commented 2 years ago

@editorialbot set 10.5281/zenodo.6390220 as archive

editorialbot commented 2 years ago

Done! Archive is now 10.5281/zenodo.6390220