[REVIEW]: mosartwmpy: A Python implementation of the MOSART-WM coupled hydrologic routing and water management model

[whedon]

Submitting author: @thurber (Travis Thurber)
Repository: https://github.com/IMMM-SFA/mosartwmpy
Version: v0.0.8
Editor: @kthyng
Reviewer: @JannisHoch, @cheginit
Archive: 10.5281/zenodo.4976463

⚠️ 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 badge code:

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

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

@JannisHoch & @cheginit, 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 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 @JannisHoch

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

• I confirm that I read and will adhere to the JOSS 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 (@thurber) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
• Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines

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: Does the paper have a section titled 'Statement of Need' that clearly states 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 @cheginit

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

• I confirm that I read and will adhere to the JOSS 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 (@thurber) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
• Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines

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: Does the paper have a section titled 'Statement of Need' that clearly states 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]

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

2. You may also like to change your default settings for this watching repositories in your GitHub profile here: https://github.com/settings/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]

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

OK DOIs

- 10.5194/hess-17-3605-2013 is OK
- 10.1175/JHM-D-12-015.1 is OK
- 10.11578/E3SM/dc.20180418.36 is OK
- 10.1016/j.cageo.2012.04.002 is OK
- 10.21105/joss.02317 is OK
- 10.1029/2020WR027902 is OK
- 10.5194/hess-23-2261-2019 is OK
- 10.1073/pnas.2020431118 is OK
- 10.5194/hess-24-1275-2020 is OK
- 10.1002/2016WR019767 is OK
- 10.5194/hess-19-3239-2015 is OK

MISSING DOIs

- None

INVALID DOIs

- None

[whedon]

Software report (experimental):

github.com/AlDanial/cloc v 1.88  T=0.69 s (82.0 files/s, 7358.3 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Python                          36            577            785           2688
YAML                             7             42            160            280
Markdown                         6             64              0            273
TeX                              1             12              0            133
DOS Batch                        1              8              1             26
reStructuredText                 2              6             10             10
make                             1              4              7              9
TOML                             1              0              0              6
Bourne Shell                     2              3              7              3
-------------------------------------------------------------------------------
SUM:                            57            716            970           3428
-------------------------------------------------------------------------------


Statistical information for the repository '53f4002b6a95f04d072abcd9' was
gathered on 2021/04/28.
The following historical commit information, by author, was found:

Author                     Commits    Insertions      Deletions    % of changes
crvernon                         6           172              5            1.11
travis                          51          9810           5927           98.89

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
crvernon                    105           61.0          2.5               15.24
travis                     3945           40.2          2.2               15.59

[whedon]

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

[whedon]

👋 @cheginit, please update us on how your review is going (this is an automated reminder).

[whedon]

👋 @JannisHoch, please update us on how your review is going (this is an automated reminder).

[crvernon]

@kthyng @cheginit Are there any questions that we could answer to help get you started with your mosartwmpy? Please don't hesitate to reach out. Thanks!

[kthyng]

Just to clarify — I'm the editor for your submission @crvernon but am not reviewing your software.

[crvernon]

Apologies @kthyng !

[cheginit]

@crvernon Hi Chris, sorry for the delay. I will post my questions by the end of this week.

[cheginit]

Overall, I find this submission a useful contribution to the hydrology community and appreciate the effort of the developers. Here are my initial thoughts on the submission.

Paper

Overall, the paper reads well and states the purpose of the project clearly. Some of my concerns have already been addressed by the new commits. However, I am not quite sure if I understand the purpose of this sentence:

As a sample benchmark, a one-year simulation using MOSART-WM on a 24-CPU node of an institutional supercomputer can be performed in about half an hour; whereas the same simulation using mosartwmpy on a personal 6-CPU laptop can be performed in about five hours.

This is not a fair and informative comparison: Running a Fortran code on a cluster with 24 CPUs vs. running a Python code on a latop with 6 CPUs. There's no common ground between the two; Code languages are different, system specifications of the testing environments are different, and number of CPUs are different. I know setting up E3SM on a laptop is not an easy task, so I would suggest to at least run the Python code on the same cluster with 8 CPUs and run the Fortran version on the same cluster and with 8 CPUs. Overall, I am not sure if this comparison is really necessary since, as you mentioned in the paper, the main purpose of mosartwmpy is prototyping and educational.

Code

The authors opted to use numexpr as the engine, so to speak, to carry out the heavy lifting in their implementation of MOSART-WM. This choice makes the code difficult to read and hinders further development by the "the domain scientist". There are much better approaches that even scale much better (more than 8 CPUs) than numexpr. I would like to know why the developers decided to use numexpr over numba?

I understand that this might be a personal preference but I believe in the future versions, the authors might want to consider refactoring their code to use numba as the engine. It can improve the performance and scaling, as well as, readability. I developed a hydrological model (not public yet) that is based on numba and have tested it on a cluster with 28 cores and it scales nicely.

Regarding the documentation, the website provides examples but I think it can be improved by providing an example for working with restart files as well as runoff outputs from CLM or E3SM outputs since this is the primary use case.

Additionally, I opened a couple of issues/suggestions in the repository.

[thurber]

Thanks @cheginit, this is good feedback!

I just merged a PR that removes most of the Functionality and limitations section based on @JannisHoch's feedback, so the section with the awkward CPU comparison is gone.

Regarding the speed and the use of numexpr -- I agree it's semantically burdensome, but at least for my own laptop and cluster, the numexpr implementation was faster than a numba implementation for doing the same vectorized calculations. That said, I'm still not satisfied with the execution speed and am planning to do an overhaul in the medium-term future. I'd be very curious to learn from your experiences with numba. In your implementation, do you solve in a vectorized fashion or cell-by-cell? Despite the traditional advice to go vectorized in Python, I fear it may have led to the current speed challenges we are facing in mosartwmpy.

Good idea about providing examples with restart files and dynamic runoff inputs -- I'll open an issue for that and get to work on it.

[cheginit]

@thurber The recommended way of implementing numba is to avoid vectorization and unroll all the loops. Adding signatures to functions can also speed up the code. You need to keep type casting during the runtime at the minimum. Another point is about the decision between using f8 or f4 that also affects the performance and accuracy.

[kthyng]

@whedon check references

[whedon]

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

OK DOIs

- 10.5194/hess-17-3605-2013 is OK
- 10.1175/JHM-D-12-015.1 is OK
- 10.11578/E3SM/dc.20180418.36 is OK
- 10.5066/F76Q1VQV is OK
- 10.1016/j.cageo.2012.04.002 is OK
- 10.21105/joss.02317 is OK
- 10.1029/2020WR027902 is OK
- 10.5194/hess-23-2261-2019 is OK
- 10.1073/pnas.2020431118 is OK
- 10.5194/hess-24-1275-2020 is OK
- 10.5194/hess-19-3239-2015 is OK
- 10.1029/2018MS001583 is OK

MISSING DOIs

- 10.5194/gmd-10-3913-2017 may be a valid DOI for title: GLOFRIM v1.0–A globally applicable computational framework for integrated hydrological–hydrodynamic modelling

INVALID DOIs

- None

[kthyng]

@whedon generate pdf

[whedon]

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

[kthyng]

@thurber Your changes look good! Does look like that is a valid missing DOI above — please add that to your Hoch reference and we should be good to go!

[thurber]

@kthyng done!

[kthyng]

@whedon check references

[whedon]

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

OK DOIs

- 10.5194/hess-17-3605-2013 is OK
- 10.1175/JHM-D-12-015.1 is OK
- 10.11578/E3SM/dc.20180418.36 is OK
- 10.5194/gmd-10-3913-2017 is OK
- 10.5066/F76Q1VQV is OK
- 10.1016/j.cageo.2012.04.002 is OK
- 10.21105/joss.02317 is OK
- 10.1029/2020WR027902 is OK
- 10.5194/hess-23-2261-2019 is OK
- 10.1073/pnas.2020431118 is OK
- 10.5194/hess-24-1275-2020 is OK
- 10.5194/hess-19-3239-2015 is OK
- 10.1029/2018MS001583 is OK

MISSING DOIs

- None

INVALID DOIs

- None

[kthyng]

@whedon recommend-accept

[whedon]

No archive DOI set. Exiting...

[kthyng]

@whedon set 10.5281/zenodo.4976463 as archive

[whedon]

OK. 10.5281/zenodo.4976463 is the archive.

[kthyng]

@whedon recommend-accept

[whedon]

Attempting dry run of processing paper acceptance...

[whedon]

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

OK DOIs

- 10.5194/hess-17-3605-2013 is OK
- 10.1175/JHM-D-12-015.1 is OK
- 10.11578/E3SM/dc.20180418.36 is OK
- 10.5194/gmd-10-3913-2017 is OK
- 10.5066/F76Q1VQV is OK
- 10.1016/j.cageo.2012.04.002 is OK
- 10.21105/joss.02317 is OK
- 10.1029/2020WR027902 is OK
- 10.5194/hess-23-2261-2019 is OK
- 10.1073/pnas.2020431118 is OK
- 10.5194/hess-24-1275-2020 is OK
- 10.5194/hess-19-3239-2015 is OK
- 10.1029/2018MS001583 is OK

MISSING DOIs

- None

INVALID DOIs

- None

[whedon]

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

Check final proof 👉 openjournals/joss-papers#2414

If the paper PDF and Crossref deposit XML look good in openjournals/joss-papers#2414, 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]

@whedon accept deposit=true

[whedon]

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

[whedon]

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

[whedon]

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

Here's what you must now do:

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

Any issues? Notify your editorial technical team...

[kthyng]

Congrats on your new publication @thurber! Thanks so much to reviewers @JannisHoch and @cheginit for your hard work, time, and expertise!! 🎉

[whedon]

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

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

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

This is how it will look in your documentation:

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:

• 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