Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[REVIEW]: NetworkSedimentTransporter: A Landlab submodule for bed material transport through river networks #2341

Closed
38 tasks done
whedon opened this issue Jun 15, 2020 · 60 comments
Assignees
Labels
accepted published Papers published in JOSS recommend-accept Papers recommended for acceptance in JOSS. review

Comments

@whedon
Copy link

whedon commented Jun 15, 2020

Submitting author: @pfeiffea (Allison Pfeiffer)
Repository: https://github.com/landlab/landlab
Version: v2.2.0
Editor: @kthyng
Reviewer: @zsylvester, @ebgoldstein
Archive: 10.5281/zenodo.4044683

⚠️ JOSS reduced service mode ⚠️

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

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

@zsylvester & @ebgoldstein, 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 @kthyng know.

Please try and complete your review in the next six weeks

Review checklist for @zsylvester

Conflict of interest

  • 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

General checks

  • Repository: Is the source code for this software available at the repository url?
  • License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
  • Contribution and authorship: Has the submitting author (@pfeiffea) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?

Functionality

  • Installation: Does installation proceed as outlined in the documentation?
  • Functionality: Have the functional claims of the software been confirmed?
  • 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

  • A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
  • Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
  • Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
  • Automated tests: Are there automated tests or manual steps described so that the functionality of the software can be verified?
  • 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

  • Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
  • A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • State of the field: Do the authors describe how this software compares to other commonly-used packages?
  • Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
  • 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 @ebgoldstein

Conflict of interest

  • 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

General checks

  • Repository: Is the source code for this software available at the repository url?
  • License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
  • Contribution and authorship: Has the submitting author (@pfeiffea) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?

Functionality

  • Installation: Does installation proceed as outlined in the documentation?
  • Functionality: Have the functional claims of the software been confirmed?
  • 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

  • A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
  • Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
  • Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
  • Automated tests: Are there automated tests or manual steps described so that the functionality of the software can be verified?
  • 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

  • Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
  • A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • State of the field: Do the authors describe how this software compares to other commonly-used packages?
  • Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
  • 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?
@whedon
Copy link
Author

whedon commented Jun 15, 2020

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @zsylvester, @ebgoldstein it looks like you're currently assigned to review this paper 🎉.

⚠️ JOSS reduced service mode ⚠️

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.

⭐ Important ⭐

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
Copy link
Author

whedon commented Jun 15, 2020

@kthyng
Copy link

kthyng commented Jul 6, 2020

Hi @zsylvester, @ebgoldstein, just want to send a friendly ping to keep this review on your radar for the next few weeks. Thanks!

@ebgoldstein
Copy link

Hi @pfeiffea — I'm walking through the tutorials now. They are very good. Two things so far:

  1. in the index to Landlab notebooks, Network Sediment Transporter is listed twice. Here is a screenshot:

Screen Shot 2020-07-13 at 11 39 56 AM

  1. I have walking through the NST tutorials using a Binder instance . Some of the links (specifically to other tutorials) point to a local port —  localhost:8888 — and therefore don't function in my Binder instance.

@ebgoldstein
Copy link

ebgoldstein commented Jul 13, 2020

Ok, I have made it through the paper, docs, tutorials, and repo. The code works as described, and is well served by 4 tutorial notebooks. I think users will be able to understand the working of the code and how to modify it for their given use case. To my eye, this is a nice addition to Landlab,

I have one suggestion: The module described is in some sense a computational replication of Czuba 2018 — and i noticed that the tutorial for a real basin was a section of the the Methow River, which is the same as the original paper. I know this is JOSS and not Rescience C, but I think a mention of the results of this replication could be made. For example, the the authors could show (as a figure) or describe (in text) or put in the tutorials a statement about the comparison to Czuba 2018 Fig2 or Fig3... even a simple statement that 'the results are identical to the code of C18'. I will leave this to @kthyng to decide if it is in scope.

also:

Nothing will stop us from making all of the choices at once.

😆

@ooo
Copy link

ooo bot commented Jul 13, 2020

👋 Hey @ebgoldstein...

Letting you know, @kthyng is currently OOO until Monday, July 20th 2020. ❤️

@kthyng
Copy link

kthyng commented Jul 20, 2020

@pfeiffea Just want to note you have some reviewer comments above.

I have one suggestion: The module described is in some sense a computational replication of Czuba 2018 — and i noticed that the tutorial for a real basin was a section of the the Methow River, which is the same as the original paper. I know this is JOSS and not Rescience C, but I think a mention of the results of this replication could be made. For example, the the authors could show (as a figure) or describe (in text) or put in the tutorials a statement about the comparison to Czuba 2018 Fig2 or Fig3... even a simple statement that 'the results are identical to the code of C18'. I will leave this to @kthyng to decide if it is in scope.

@ebgoldstein and @pfeiffea — This sounds like a small and reasonable request, but happy to hear if otherwise.

@kthyng
Copy link

kthyng commented Jul 20, 2020

@zsylvester Would you be able to work on this review in the next two weeks?

@zsylvester
Copy link

Yes, I will try to get to it this week. I just realized that I have not 'officially' accepted the invitation yet - when I click on the link (https://github.com/openjournals/joss-reviews/invitations) it says that it has expired.

@zsylvester
Copy link

@kthyng It still does not let me to accept the invitation / go through the checklist.

@kthyng
Copy link

kthyng commented Jul 21, 2020

@whedon re-invite @zsylvester as reviewer

@whedon
Copy link
Author

whedon commented Jul 21, 2020

OK, the reviewer has been re-invited.

@zsylvester please accept the invite by clicking this link: https://github.com/openjournals/joss-reviews/invitations

@kthyng
Copy link

kthyng commented Jul 21, 2020

@zsylvester This should do it for you, try the link above.

@zsylvester
Copy link

I have gone through the paper, the four Jupyter notebooks, scanned the repo, looked at the documentation, and ran the tests. I think this is a nice addition to the Landlab package and it is likely to be of interest to anybody who is working on sediment transport problems in primarily non-depositional landscapes. The NetworkSedimentTransporter component is installed along with the Landlab package and I had no issues installing Landlab 2.0 (that happened a while ago). The three notebooks ran smoothly on Binder and they are easy to follow along, I can see how the second one could form a starting point for playing with someone's own dataset. I have run the fourth one locally.

Here are a few comments / suggestions:

  • I agree with @ebgoldstein that a statement about how the model compares with Czuba (2018) would be useful.
  • Along the same lines, it would be nice to see a bit larger / more complex model in the notebook with the shapefile example, maybe something similar to what is shown in Czuba (2018), and show how it could be used to provide insights into some aspects of sediment transport.
  • The profiling and scaling notebook could use a bit more explanation / documentation.
  • Many of the links in the notebooks do not work.
  • Link to Jupyter notebook at the end of the manuscript does not work.
  • I think 'MATLAB' is the correct usage, not 'Matlab'.
  • 'computational infrastructure built in order to create the NetworkSedimentTransporter' instead of 'computational infrastructure created in order to create the NetworkSedimentTransporter'.
  • '...required the addition of two new data structures' instead of '...required the creation of two new data structures'.
  • 'Landlab' instead of 'landlab' in the Hobley et al. reference.

@kthyng
Copy link

kthyng commented Aug 11, 2020

@pfeiffea How are your changes coming along? I think the reviews are waiting on you currently.

@pfeiffea
Copy link

@kthyng Changes are nearly complete-- we're just waiting for them to be published in the newest release of landlab/landlab. We should have a new draft to you all in the next few days.

@pfeiffea
Copy link

@kthyng, @ebgoldstein and @zsylvester:

Thank you for your thorough reading and suggestions on the first draft of our submission. We have edited the component, notebooks, and text. The updated version is now published on the main branch of Landlab (v2.2.0): https://github.com/landlab/landlab

Our response to individual comments are included below.

A) Computational Replication / Comparison to Czuba (2018)

Both reviewers bring up making a clearer comparison between this paper and the work presented in Czuba (2018). Specifically the following two comments:

From @ebgoldstein

I have one suggestion: The module described is in some sense a computational replication of Czuba 2018 — and i noticed that the tutorial for a real basin was a section of the the Methow River, which is the same as the original paper. I know this is JOSS and not Rescience C, but I think a mention of the results of this replication could be made. For example, the the authors could show (as a figure) or describe (in text) or put in the tutorials a statement about the comparison to Czuba 2018 Fig2 or Fig3... even a simple statement that 'the results are identical to the code of C18'. I will leave this to @kthyng to decide if it is in scope.

From @zsylvester:

I agree with @ebgoldstein that a statement about how the model compares with Czuba (2018) would be useful.

Response:
The NetworkSedimentTransporter is not a direct translation of the model presented in Czuba (2018) and thus we don't think an attempt at replicating the results of this paper makes sense. However, we interpret these comments to indicate two important points: (a) that our original text does not make clear that this code is not a direct translation of Czuba (2018) and (b) the specific differences between this implementation and that of Czuba (2018) are not sufficiently clear.

We felt that the best way to address this was to add the following text:

Our implementation is not a direct translation of the model implemented in MATLAB and described in @Czuba2018. In addition, we add two new elements to the model: variable sediment density and bed-material abrasion, and specification of active layer thickness.

We also revised the text describing the differences for clarity.

B) Profiling and Scaling Notebook

@zsylvester pointed out the bare-bones nature of the profiling and scaling notebook:

The profiling and scaling notebook could use a bit more explanation / documentation.

Thank you for pointing out this oversight. We have greatly expanded this notebook. It should be far more helpful to users in its revised form.

C) A More Complex Model Notebook

@zsylvester made a request for a notebook demonstrating a more sophisticated use case:

[It] would be nice to see a bit larger / more complex model in the notebook with the shapefile example, maybe something similar to what is shown in Czuba (2018), and show how it could be used to provide insights into some aspects of sediment transport.

We feel that a more complex model would be out of scope for a Landlab tutorial. The goal of the Landlab tutorials is to demonstrate a simple, generalized use case of each component, rather than to explore specific scientific questions. Previous work on network-scale Lagrangian sediment transport has demonstrated the value of these models to our understanding of basin scale sediment dynamics (see Gran and Czuba, 2017 , Czuba and Foufoula-Georgiou, 2015 ). Furthermore, the updated profiling and scaling notebook will provide potential users a sense for the efficiency of this component for larger models.

D) New Active Layer Options

The original version of the paper text referenced multiple methods for determining active layer thickness:

Many sediment transport models [e.g., @Cui2007b; @Czuba2018] represent the mobile portion of the grains on the riverbed at any given time as an "active layer" of constant thickness. All grains in this layer are transported, whereas all grains below this layer are immobile. Within NetworkSedimentTransporter the user has the option to specify active layer thickness as a constant value or a multiple of the mean grain size in each link. Alternatively, we incorporated the formulation of @Wongetal2007 to calculate an active layer thickness for each link in the network at each timestep as a function of Shields stress and median grain diameter.

We realized that this functionality was not included in the version of the code that was originally merged into Landlab and submitted to JOSS. We have included this ‘knob’ (and associated tests) in this round of edits.

E) Term used to describe NST

After a discussion between authors, we have decided to call the NetworkSedimentTransporter a “component” instead of “submodule”. The difference is semantic, though we think that “component” may be clearer for readers.

F) Minor Edits

We have fixed the typos, duplicate notebook listings, and broken links pointed out by @ebgoldstein and @zsylvester. Thank you for finding these!

@kbarnhart
Copy link

@whedon check references

@kbarnhart
Copy link

@whedon generate pdf

@whedon
Copy link
Author

whedon commented Aug 18, 2020

@kbarnhart
Copy link

@pfeiffea I just checked the PDF and found a few formatting things to fix (e.g., reference capitalization, @mcflugen's affiliation was incorrect). Am fixing it and will update here when done.

@kbarnhart
Copy link

@whedon generate pdf from branch barnhark/small_joss_fixes_nst

@whedon
Copy link
Author

whedon commented Aug 18, 2020

Attempting PDF compilation from custom branch barnhark/small_joss_fixes_nst. Reticulating splines etc...

@whedon
Copy link
Author

whedon commented Aug 18, 2020

@kbarnhart
Copy link

@whedon generate pdf

@zsylvester
Copy link

@kthyng @pfeiffea Everything looks fine to me!

@kthyng
Copy link

kthyng commented Sep 21, 2020

Ok then, it looks like the reviewers have both recommended this publication for acceptance. We can start the final bits to wrap up now.

@kthyng
Copy link

kthyng commented Sep 21, 2020

@whedon check references

@whedon
Copy link
Author

whedon commented Sep 21, 2020

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

OK DOIs

- 10.5194/esurf-5-21-2017 is OK
- 10.5194/esurf-2020-12 is OK
- 10.1016/j.geomorph.2018.08.031 is OK
- 10.1002/esp.4195 is OK
- 10.1002/2014WR016862 is OK
- 10.1002/2016JF003965 is OK
- 10.1002/esp.4635 is OK
- 10.1080/00221686.2006.9521683 is OK
- 10.1080/00221686.2006.9521684 is OK
- 10.1002/rra.1012 is OK
- 10.1061/(ASCE)0733-9429(2003)129:2(120) is OK
- 10.1029/2006WR005330 is OK
- 10.1029/97WR02387 is OK
- 10.1002/2015JF003491 is OK
- 10.1029/2006WR005172 is OK

MISSING DOIs

- None

INVALID DOIs

- None

@kthyng
Copy link

kthyng commented Sep 21, 2020

@whedon generate pdf

@whedon
Copy link
Author

whedon commented Sep 21, 2020

👉📄 Download article proof 📄 View article proof on GitHub 📄 👈

@kthyng
Copy link

kthyng commented Sep 21, 2020

Paper looks great!

Next, @pfeiffea can you verify the version of software you want tagged with this publication?

After that, please upload an archive of your software to something like Zenodo. When doing so, please make sure that the title of author list of the archive exactly match your JOSS paper. Then, report back here with the doi.

@pfeiffea
Copy link

pfeiffea commented Sep 23, 2020

Thanks, @kthyng!

Landlab v2.2.0

Here's the doi for a Zenodo archive: 10.5281/zenodo.4044683

@kthyng
Copy link

kthyng commented Sep 24, 2020

@whedon set v2.2.0 as version

@whedon
Copy link
Author

whedon commented Sep 24, 2020

OK. v2.2.0 is the version.

@kthyng
Copy link

kthyng commented Sep 24, 2020

@pfeiffea You have a small typo in your title for the Zenodo archive (forbed instead of for bed) — can you fix this?

@pfeiffea
Copy link

Thanks for catching that @kthyng

Fixed!

@kthyng
Copy link

kthyng commented Sep 24, 2020

@whedon set 10.5281/zenodo.4044683 as archive

@whedon
Copy link
Author

whedon commented Sep 24, 2020

OK. 10.5281/zenodo.4044683 is the archive.

@kthyng
Copy link

kthyng commented Sep 24, 2020

@whedon accept

@whedon
Copy link
Author

whedon commented Sep 24, 2020

Attempting dry run of processing paper acceptance...

@whedon whedon added the recommend-accept Papers recommended for acceptance in JOSS. label Sep 24, 2020
@whedon
Copy link
Author

whedon commented Sep 24, 2020

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

Check final proof 👉 openjournals/joss-papers#1751

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

@whedon accept deposit=true

@kthyng
Copy link

kthyng commented Sep 24, 2020

@whedon accept deposit=true

@whedon
Copy link
Author

whedon commented Sep 24, 2020

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

@whedon whedon added accepted published Papers published in JOSS labels Sep 24, 2020
@whedon
Copy link
Author

whedon commented Sep 24, 2020

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

@whedon
Copy link
Author

whedon commented Sep 24, 2020

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

Here's what you must now do:

  1. Check final PDF and Crossref metadata that was deposited 👉 Creating pull request for 10.21105.joss.02341 joss-papers#1752
  2. Wait a couple of minutes to verify that the paper DOI resolves https://doi.org/10.21105/joss.02341
  3. If everything looks good, then close this review issue.
  4. Party like you just published a paper! 🎉🌈🦄💃👻🤘

Any issues? Notify your editorial technical team...

@kthyng
Copy link

kthyng commented Sep 24, 2020

Congrats on your new publication @pfeiffea! Many thanks to reviewers @zsylvester and @ebgoldstein for your time, expertise, and hard work. We couldn't do this without you!

(I will leave this issue open until the DOI resolves)

@kthyng kthyng closed this as completed Sep 24, 2020
@whedon
Copy link
Author

whedon commented Sep 24, 2020

🎉🎉🎉 Congratulations on your paper acceptance! 🎉🎉🎉

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

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

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

This is how it will look in your documentation:

DOI

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:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted published Papers published in JOSS recommend-accept Papers recommended for acceptance in JOSS. review
Projects
None yet
Development

No branches or pull requests

6 participants