search

openjournals / joss-reviews

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

[REVIEW]: SubZero: a Discrete element sea ice model that simulates floes as evolving concave polygons #5039

Closed editorialbot closed 1 year ago

editorialbot commented 1 year ago

Submitting author: !--author-handle-->@bpm5026<!--end-author-handle-- (Brandon Montemuro) Repository: https://github.com/SeaIce-Math/SubZero Branch with paper.md (empty if default branch): JOSS Version: v1.0.4 Editor: !--editor-->@hugoledoux<!--end-editor-- Reviewers: @ZijieZhaoMMHW, @zhaohuiwang-imas Archive: 10.5281/zenodo.8205778

Status

status

Status badge code:

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

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

@ZijieZhaoMMHW & @zhaohuiwang-imas, 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 @hugoledoux 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 @ZijieZhaoMMHW

📝 Checklist for @ZhaohuiWang-imas

editorialbot commented 1 year 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 1 year ago 
Software report:

github.com/AlDanial/cloc v 1.88  T=0.26 s (644.9 files/s, 158944.8 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Pascal                           7            585            705           6235
C++                              7            698            619           5598
C#                              11            558            701           5461
MATLAB                          43            860           1169           4059
CSS                             18             63            273           3752
JavaScript                      35            553           1099           2405
Python                           2            261            129           2136
XML                              8             22             96            753
C/C++ Header                     3             85             97            578
Sass                            18            103             32            472
SVG                              3              1              1            459
MSBuild script                   3              0             21            271
TeX                              1             22              0            217
Markdown                         3             53              0             94
HTML                             2              3              0             31
YAML                             1              1              4             18
CMake                            1              5              1             15
Ruby                             1              1              2             11
make                             1              7              3             10
-------------------------------------------------------------------------------
SUM:                           168           3881           4952          32575
-------------------------------------------------------------------------------

gitinspector failed to run statistical information for the repository
editorialbot commented 1 year ago

Wordcount for paper.md is 845

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:

editorialbot commented 1 year ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1525/elementa.304 is OK
- 10.3189/2013AoG62A055 is OK
- 10.5281/zenodo.7335901 is OK

MISSING DOIs

- 10.1029/jc082i027p03932 may be a valid DOI for title: A viscous sea ice law as a stochastic average of plasticity
- 10.1007/3-540-26970-3_4 may be a valid DOI for title: Sea ice rheology
- 10.1029/2005jc003393 may be a valid DOI for title: Arctic Ice Dynamics Joint Experiment (AIDJEX) assumptions revisited and found inadequate
- 10.5194/tc-10-1055-2016 may be a valid DOI for title: neXtSIM: a new Lagrangian sea ice model
- 10.5194/tc-14-93-2020 may be a valid DOI for title: Feature-based comparison of sea ice deformation in lead-permitting sea ice simulations
- 10.1029/2021ms002523 may be a valid DOI for title: Simulating Linear Kinematic Features in Viscous-Plastic Sea Ice Models on Quadrilateral and Triangular Grids With Different Variable Staggering
- 10.1680/geot.1979.29.1.47 may be a valid DOI for title: A discrete numerical model for granular assemblies
- 10.31223/osf.io/j6vpn may be a valid DOI for title: Application of discrete element methods to approximate sea ice dynamics
- 10.1029/2021ms002513 may be a valid DOI for title: Lagrangian Data Assimilation and Parameter Estimation of an Idealized Sea Ice Discrete Element Model
- 10.1016/j.apor.2018.02.022 may be a valid DOI for title: Ice load on floating structure simulated with dilated polyhedral discrete element method in broken ice field
- 10.31223/x5rm0f may be a valid DOI for title: SubZero: A Sea Ice Model with an Explicit Representation of the Floe Life Cycle

INVALID DOIs

- None
ZijieZhaoMMHW commented 1 year ago

Review checklist for @ZijieZhaoMMHW

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

ZijieZhaoMMHW commented 1 year ago

Sorry for the delay in review. I have been busy with workloads and personal health issues last month, but I am close to finishing the review now and will try to finish it this weekend.

ZijieZhaoMMHW commented 1 year ago

Sorry for the delay in reviewing. My comments are provided as follows: @hugoledoux @bpm5026

Using decent parameterization and correspondences to previous literature, this versatile toolbox offers an idealized depiction of floe life cycles. To confirm its practicality, I conducted multiple tests following the instructions on the GitHub page and utilizing the three validation cases provided by the authors. As a result, I verified the efficacy of this toolbox and I think its associated paper can be published in the JOSS. However, I do have some suggestions to improve the toolbox from the view of users. My comments are listed as follows:

  1. Software environment: I have run the toolbox on two MAC systems, but I have observed that XCode is required to run the toolbox on this platform. Consequently, I am uncertain whether the program can function correctly on Windows. Can you please verify this?
  2. Validation cases: The "validation cases" file contains three comprehensive cases provided by the authors. Although these cases can be executed correctly, they can be quite challenging for novice users like myself who have limited experience with idealized floe simulations and sea ice. Therefore, I suggest that the authors provide step-by-step instructions for the three validation cases in a "readme" file, as many other authors have done in their toolboxes published in JOSS. Additionally, this paper seems to be the toolbox version of the paper “SubZero: A Sea Ice Model with an Explicit Representation of the Floe Life Cycle” published in JAMES. If you can use this toolbox to reproduce results in the source paper it can be very helpful for new users.
  3. Documents: There are some typo errors in the documents provided in the toolbox, like the “MATLBA” in the “Installation instructions”. Please correct them. Additionally, a “change history” document file is required to record the update and changing history of the toolbox.
  4. Community guideline: Please include a document file to indicate how others can contribute to the further development and update of this toolbox and list it in the readme file.

My proficiency in idealized floe simulations is restricted, so during the review, I focused on verifying that the toolbox functioned correctly according to the authors' description and that the resulting outputs were scientifically reasonable, both of which were satisfactory. However, I am unable to assess whether the toolbox has been coded in a scientifically sound manner or if there are any inaccuracies associated with the presented floe simulations. For this matter, it would be appropriate to await comments from the other reviewer and feedback from users in the future.

hugoledoux commented 1 year ago

@bpm5026 besides the comments above, I would add that @ZijieZhaoMMHW didn't click the "automated tests" checkbox, and that this should be added (or justify why this is not possible)

bpm5026 commented 1 year ago

@hugoledoux @ZijieZhaoMMHW Thank you both for your reviews! I am currently swamped with some grading but I am hopeful that I should be able to move to addressing these comments by the end of this week/early next week.

hugoledoux commented 1 year ago

@bpm5026 any updates?

bpm5026 commented 1 year ago

@hugoledoux Sorry for the delay, I have addressed the majority of the review comments. I just underestimated the amount of time it would take to write the automated test. I have most of the automated test written and will be finished with revisions once that is done. I think this should be finished relatively soon (hopefully by the end of the weekend) and will let you know once I am done.

bpm5026 commented 1 year ago

@hugoledoux I got the automated test working, so tomorrow I am going to update everything in the GitHub repository, go through all the comments again to make sure I addressed everything, and then it should be done.

bpm5026 commented 1 year ago

@hugoledoux @ZijieZhaoMMHW Thank you again for the thoughtful reviews; here is what has been addressed. Let me know if there are any additional comments or if you feel I have not adequately addressed the original comments. 1) When we tested the program to function correctly on Windows, it ran fine without any additional installations similar to Xcode on the mac. 2) Step-by-step instructions have been added to the README file for modifying the base code to run the validation cases. 3) Updated typographic errors in the readme and added CHANGES.md file to the repository. 4) Added CONTRIBUTING.md file with instructions. 5) Added automated test, which tests to ensure that collisions do not generate energy.

hugoledoux commented 1 year ago

@ZhaohuiWang-imas we are waiting for you to start the review, when would that be possible?

hugoledoux commented 1 year ago

@ZijieZhaoMMHW above (https://github.com/openjournals/joss-reviews/issues/5039#issuecomment-1495063871) you put a thumbsup, does it mean you are happy with the modifications or that you will verify them later?

ZijieZhaoMMHW commented 1 year ago

@ZijieZhaoMMHW above (#5039 (comment)) you put a thumbsup, does it mean you are happy with the modifications or that you will verify them later?

I am currently doing the verification and going to finish it this week. Will post another comment after that.

ZijieZhaoMMHW commented 1 year ago

@hugoledoux @bpm5026

Having completed additional verification, I am delighted to ensure the release of this toolbox and the publication of corresponding paper, which represents a valuable contribution to the implementation of discrete element simulation in sea ice. Congratulations.

bpm5026 commented 1 year ago

Thank you @ZijieZhaoMMHW for the thorough review! You brought up many good points that helped improve the package.

hugoledoux commented 1 year ago

@ZhaohuiWang-imas we are waiting for you to start the review, when would that be possible?

Could you please check that everything is working by generating your checklist? @editorialbot generate my checklist

bpm5026 commented 1 year ago

@hugoledoux Is there anything we can do to keep this moving forward?

ZhaohuiWang-imas commented 1 year ago

@hugoledoux @bpm5026 Sorry for the delay. Sorry for my misunderstandings regarding previous information during communicating. I will start the review before the end of this week. Is that time acceptable?

hugoledoux commented 1 year ago

Is that time acceptable?

yes.

bpm5026 commented 1 year ago

@ ZhaohuiWang-imas Do you have any comments so far?

ZhaohuiWang-imas commented 1 year ago

Review checklist for @ZhaohuiWang-imas

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

ZhaohuiWang-imas commented 1 year ago

Review of "SubZero: a Discrete element sea ice model that simulates floes as evolving concave polygons"

This article introduces a new discrete element sea ice model that utilises parameterisations of floe-scale processes. Since the theoretical manuscript of the model have already been published in JAMES and undergone peer review, this revision focused primarily on the feasibility of the model described in the article. Compared to traditional sea ice models commonly written in Fortran, this model runs on Matlab, making it easier to understand, use, and modify. It greatly simplifies the entry barrier for beginners. The program effectively distinguishes between the user-defined areas and the code areas that users do not need to touch. Upon testing, the program is plug-and-play and runs smoothly. The base program runs at a reasonable speed (tested on a 2021 MacBook Pro M1 Max chip). The three validation cases provided by the author can all be executed, and the results are as expected. The application guide (README) for the program is written in a clear and understandable manner, with a logical structure. I believe this model is very innovative, and the research findings are at the cutting edge. The author's open-access approach to the software and the clear user guide make a significant contribution to the field. Therefore, I recommend that your journal publish this important achievement.

Results: Accept for publication without revision.

bpm5026 commented 1 year ago

Thank you @ZhaohuiWang-imas, I appreciate the time and effort put into your thorough review. @hugoledoux Let me know if there is anything else you need me to do to keep this moving along.

hugoledoux 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:

hugoledoux commented 1 year ago

@editorialbot check references

editorialbot commented 1 year ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1525/elementa.304 is OK
- 10.3189/2013AoG62A055 is OK
- 10.5281/zenodo.7335901 is OK

MISSING DOIs

- 10.1029/jc082i027p03932 may be a valid DOI for title: A viscous sea ice law as a stochastic average of plasticity
- 10.1007/3-540-26970-3_4 may be a valid DOI for title: Sea ice rheology
- 10.1029/2005jc003393 may be a valid DOI for title: Arctic Ice Dynamics Joint Experiment (AIDJEX) assumptions revisited and found inadequate
- 10.5194/tc-10-1055-2016 may be a valid DOI for title: neXtSIM: a new Lagrangian sea ice model
- 10.5194/tc-14-93-2020 may be a valid DOI for title: Feature-based comparison of sea ice deformation in lead-permitting sea ice simulations
- 10.1029/2021jc017666 may be a valid DOI for title: Sea Ice Rheology Experiment (SIREx): 2. Evaluating Linear Kinematic Features in High-Resolution Sea Ice Simulations
- 10.1029/2021ms002523 may be a valid DOI for title: Simulating Linear Kinematic Features in Viscous-Plastic Sea Ice Models on Quadrilateral and Triangular Grids With Different Variable Staggering
- 10.1680/geot.1979.29.1.47 may be a valid DOI for title: A discrete numerical model for granular assemblies
- 10.31223/osf.io/j6vpn may be a valid DOI for title: Application of discrete element methods to approximate sea ice dynamics
- 10.1029/2021ms002513 may be a valid DOI for title: Lagrangian Data Assimilation and Parameter Estimation of an Idealized Sea Ice Discrete Element Model
- 10.1016/j.apor.2018.02.022 may be a valid DOI for title: Ice load on floating structure simulated with dilated polyhedral discrete element method in broken ice field
- 10.31223/x5rm0f may be a valid DOI for title: SubZero: A Sea Ice Model with an Explicit Representation of the Floe Life Cycle

INVALID DOIs

- None
hugoledoux commented 1 year ago

Thanks everyone, it took some time but we got there in the end :)

@ZhaohuiWang-imas: you checked all but one check (State of the field) but I believe this is a mistake? The paper clearly has that part written, so I'll move on to accept it.

Before we continue though, 4 things need to be done:

  1. accept the PR I just made: https://github.com/SeaIce-Math/SubZero/pull/5
  2. Fix the DOIs! At JOSS we like DOIs and we want all papers to have a DOI, so please follow the tips from our bot and/or find the DOIs and add them to the bibtex file.
  3. one potential typo in the paper, line 48: shouldn't it be "CONVEX polygonal elements"? I mean, the Voronoi are convex, and after the first collisions the polygons all look convex to me
  4. line 19 + line 22: I am a layman in your field, but why the "O" for the scale? Is that a normal nomenclature? I found that confusing, but perhaps it's me 🤷‍♂️
bpm5026 commented 1 year ago

@hugoledoux Thank you for your help, time and effort. See responses below to your comments: 1) Done 2) Added DOI to all the references that have them. 3) One of the new things about this model is we allow for concave polygons; you can see in fig 1a that the floes near the coastline are indeed concave. Then it is true that in this simulation, as they evolve, they tend to become convex, but that doesn't happen in others. 4) The big 'O' is a notation that indicates something is "on the order of," so in this case it means the floes are order 100 kilometers in size. If you would prefer that be spelled out more for this journal, let me know and I will change that.

hugoledoux commented 1 year ago

@editorialbot check reference

editorialbot commented 1 year ago

I'm sorry human, I don't understand that. You can see what commands I support by typing:

@editorialbot commands

hugoledoux commented 1 year ago

@editorialbot check references

editorialbot commented 1 year ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1525/elementa.304 is OK
- 10.3189/2013AoG62A055 is OK
- 10.5281/zenodo.7335901 is OK

MISSING DOIs

- None

INVALID DOIs

- https://doi.org/10.1029/JC082i027p03932 is INVALID because of 'https://doi.org/' prefix
- https://doi.org/10.1175/1520-0485(1979)009<0815:ADTSIM>2.0.CO;2 is INVALID because of 'https://doi.org/' prefix
- https://doi.org/10.1146/annurev.fluid.40.111406.102151 is INVALID because of 'https://doi.org/' prefix
- https://doi.org/10.1029/2005JC003393 is INVALID because of 'https://doi.org/' prefix
- https://doi.org/10.1175/1520-0485(1997)027<1849:AEVPMF>2.0.CO;2 is INVALID because of 'https://doi.org/' prefix
- https://doi.org/10.5194/tc-10-1055-2016 is INVALID because of 'https://doi.org/' prefix
- https://doi.org/10.1029/JC089iC04p06477 is INVALID because of 'https://doi.org/' prefix
- https://doi.org/10.1002/2015JC010770 is INVALID because of 'https://doi.org/' prefix
- https://doi.org/10.5194/tc-14-93-2020 is INVALID because of 'https://doi.org/' prefix
- https://doi.org/10.1175/MWR-D-19-0359.1 is INVALID because of 'https://doi.org/' prefix
- https://doi.org/10.1029/2021JC017667. is INVALID because of 'https://doi.org/' prefix
- https://doi.org/10.1029/2021MS002523 is INVALID because of 'https://doi.org/' prefix
- https://doi.org/10.1680/geot.1979.29.1.47 is INVALID because of 'https://doi.org/' prefix
- https://doi.org/10.1029/2018MS001299 is INVALID because of 'https://doi.org/' prefix
- https://doi.org/10.1029/2021MS002513 is INVALID because of 'https://doi.org/' prefix
- https://doi.org/10.1016/j.apor.2018.02.022 is INVALID because of 'https://doi.org/' prefix
- https://doi.org/10.1029/2022MS003247 is INVALID because of 'https://doi.org/' prefix
- https://doi.org/10.1016/j.ijrmms.2004.09.011 is INVALID because of 'https://doi.org/' prefix
hugoledoux commented 1 year ago

@bpm5026

  1. you need to remove the 'http://doi.org/' prefixes, see above
  2. okay clear, maybe a sentence about this in the paper would help the reader? Not a necessity, but I think it would be nice.
  3. Never seen the big-oh in that context (more O(n^2)), but it's fine, was just checking
bpm5026 commented 1 year ago

@hugoledoux

  1. Removed the 'http://doi.org/' prefixes
  2. Added 'In this simulation, the complex initial floe shapes near the land become simpler and more convex as they break up.' to the figure caption as well as the sentence 'Floes undergo many processes that affect their shapes, including fracturing, ridging/rafting, and welding, making them concave.' to the last paragraph before the mention of concave there.
  3. Tweaked sentence so it says 'At length scales O(10--100) km and smaller (where O designates the order of magnitude), sea ice exhibits granular behavior as individual floes and fracture networks become evident' to be clear what we mean here to readers.
hugoledoux commented 1 year ago

@editorialbot check references

editorialbot commented 1 year ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1175/1520-0485(1979)009<0815:ADTSIM>2.0.CO;2 is OK
- 10.1146/annurev.fluid.40.111406.102151 is OK
- 10.1029/2005JC003393 is OK
- 10.1175/1520-0485(1997)027<1849:AEVPMF>2.0.CO;2 is OK
- 10.5194/tc-10-1055-2016 is OK
- 10.1029/JC089iC04p06477 is OK
- 10.1002/2015JC010770 is OK
- 10.1525/elementa.304 is OK
- 10.5194/tc-14-93-2020 is OK
- 10.1175/MWR-D-19-0359.1 is OK
- 10.1029/2021MS002523 is OK
- 10.1680/geot.1979.29.1.47 is OK
- 10.1029/2018MS001299 is OK
- 10.1029/2021MS002513 is OK
- 10.1016/j.apor.2018.02.022 is OK
- 10.3189/2013AoG62A055 is OK
- 10.1029/2022MS003247 is OK
- 10.5281/zenodo.7335901 is OK
- 10.1016/j.ijrmms.2004.09.011 is OK

MISSING DOIs

- None

INVALID DOIs

- 0.1029/JC082i027p03932 is INVALID
- 10.1029/2021JC017667. is INVALID
hugoledoux commented 1 year ago

What I should have said is: modify the DOIs and run @editorialbot check references, there are two small errors still, see above. 1) it should start with "1" (so "10.1029..."); 2) remove the trailing dot

At this point could you:

I can then move forward with recommending acceptance of the submission.

bpm5026 commented 1 year ago

@editorialbot check references

editorialbot commented 1 year ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1029/JC082i027p03932 is OK
- 10.1175/1520-0485(1979)009<0815:ADTSIM>2.0.CO;2 is OK
- 10.1146/annurev.fluid.40.111406.102151 is OK
- 10.1029/2005JC003393 is OK
- 10.1175/1520-0485(1997)027<1849:AEVPMF>2.0.CO;2 is OK
- 10.5194/tc-10-1055-2016 is OK
- 10.1029/JC089iC04p06477 is OK
- 10.1002/2015JC010770 is OK
- 10.1525/elementa.304 is OK
- 10.5194/tc-14-93-2020 is OK
- 10.1175/MWR-D-19-0359.1 is OK
- 10.1029/2021JC017667 is OK
- 10.1029/2021MS002523 is OK
- 10.1680/geot.1979.29.1.47 is OK
- 10.1029/2018MS001299 is OK
- 10.1029/2021MS002513 is OK
- 10.1016/j.apor.2018.02.022 is OK
- 10.3189/2013AoG62A055 is OK
- 10.1029/2022MS003247 is OK
- 10.5281/zenodo.7335901 is OK
- 10.1016/j.ijrmms.2004.09.011 is OK

MISSING DOIs

- None

INVALID DOIs

- None
bpm5026 commented 1 year ago

1) Fixed the 2 DOIs (See above) 2) The release with JOSS is SubZero v1.0.4: https://github.com/SeaIce-Math/SubZero/releases/tag/v1.0.4 3) Zenodo archive: https://zenodo.org/record/8205778 4) Zenodo title matches paper, authors are listed, and author ORCID IDs are correct 5) Zenodo DOI: 10.5281/zenodo.8205778

Let me know if there is anything missing

hugoledoux 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:

hugoledoux commented 1 year ago

4. Zenodo title matches paper, authors are listed, and author ORCID IDs are correct

no they don't, please fix:

  1. remove the capital "D" from "Discrete" the title in zenodo
  2. the middle name of both authors need to be there in zenodo (or removed from the paper since your ORCID doesn't have them, your choice).
bpm5026 commented 1 year ago

Sorry, I missed those little details. I changed it to "discrete" and added the middle initials to Zenodo.

hugoledoux commented 1 year ago

@editorialbot set 10.5281/zenodo.8205778 as archive