[REVIEW]: G³M-f a global gradient-based groundwater modelling framwork

[whedon]

Submitting author: @rreinecke (Robert Reinecke)
Repository: https://github.com/rreinecke/global-gradient-based-groundwater-model
Version: v1.0.0
Editor: @Kevin-Mattheus-Moerman
Reviewer: @wkearn
Archive: 10.5281/zenodo.1175540

Status

Status badge code:

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

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) 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

@wkearn, 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.theoj.org/about#reviewer_guidelines. Any questions/concerns please let @Kevin-Mattheus-Moerman know.

Conflict of interest

• As the reviewer I confirm that I have read the JOSS conflict of interest policy and that there are no conflicts of interest for me to review this work.

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?
• Version: Does the release version given match the GitHub release (v1.0.0)?
• Authorship: Has the submitting author (@rreinecke) 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 function 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

• Authors: Does the paper.md file include a list of authors with their affiliations?
• A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
• References: Do all archival references that should have a DOI list one (e.g., papers, datasets, software)?

[whedon]

Hello human, I'm @whedon. I'm here to help you with some common editorial tasks. @wkearn it looks like you're currently assigned as the reviewer for this paper 🎉.

⭐ 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

[whedon]

Attempting PDF compilation. Reticulating splines etc...

[whedon]

PDF failed to compile for issue #548 with the following error: 

 Can't find any papers to compile :-(

[Kevin-Mattheus-Moerman]

@rreinecke this is where the review will take place

[Kevin-Mattheus-Moerman]

@rreinecke can you change the name of your paper file to use small letters (rather than capital letters). I think this might be why it isn't finding it.

[rreinecke]

@Kevin-Mattheus-Moerman done

[Kevin-Mattheus-Moerman]

@whedon generate pdf

[whedon]

Attempting PDF compilation. Reticulating splines etc...

[whedon]

https://github.com/openjournals/joss-papers/blob/joss.00548/joss.00548/10.21105.joss.00548.pdf

[Kevin-Mattheus-Moerman]

@rreinecke That worked. Btw you can also ask whedon to generate the PDF yourself. I noticed there are several DOI's missing for your references. Can you add them?

[rreinecke]

@whedon generate pdf

[whedon]

Attempting PDF compilation. Reticulating splines etc...

[whedon]

https://github.com/openjournals/joss-papers/blob/joss.00548/joss.00548/10.21105.joss.00548.pdf

[Kevin-Mattheus-Moerman]

@rreinecke @wkearn how are things? Can you provide an update on where things stand? Thanks. 🐨

[wkearn]

I'm working on this right now. I plan to be finished with my review by tomorrow or Wednesday at the latest. Sorry it's taken so long to get going.

[Kevin-Mattheus-Moerman]

@wkearn that is fine thanks for the update. 👍

[wkearn]

Here is my review!

G³M seems like a powerful groundwater modeling framework with a lot of potential for becoming a reusable component in hydrological models at a variety of scales. My main concern is that under the current state of its documentation, it would be very challenging for a new user to start running their own models or to couple G³M to other models.

I also have a few minor concerns about the installation process, some of which might not be bugs in G³M, but in dependencies.

Documentation

Governing equations, etc.

I would like to see a summary of the governing equations, the boundary conditions, and the numerical methods behind this model. This would be really helpful in comparing it to other models (say MODFLOW).

The simple model example

A walkthrough of the simple model would be a great help in understanding exactly what a user would need to construct their own model. How would a hydrologist go from 0 to output that they can analyze? I think you have the building blocks of this in your current documentation (such as the configuration file example), but some more explanation of what each of the options controls and how to implement the GW_Interface would go a long way towards making it easier to get started.

A more complex example

It would also be good to see a more complex example that exercises more of the software options, especially coupling G³M to other models, since that seems to be a key argument for G³M.

Installation

Dependencies are not automatically installed, which is particularly tricky for Google Test, which now recommends including libgtest in the project, rather than relying on a precompiled shared library (so that there isn't a libgtest package in apt, for instance). I built libgtest on my own and then copied it into /usr/local/lib, which is a complicated thing for a new user to be asked to do without further instruction. This could be resolved by either distributing Google Test with the project, by providing instructions for the user to put the Google Test code into the project themselves (setting appropriate environment variables) or by modifying the build process so that a user can build the models without building the tests.

I am having trouble compiling the tests because of what might be a problem with my Boost installation. I can comment out the offending lines in tagged_integer.hpp, and the tests compile and run properly. I am more concerned that this causes the documented build process to fail.

This is the error that I get. If this is a Boost bug, I'd be happy to run it down with the Boost developers. I can also open an issue on the G³M repository if you would like to handle it over there.

[ 95%] Building CXX object CMakeFiles/runUnitTests.dir/tests/testrunner.cpp.o
In file included from /usr/include/boost/log/sinks/attribute_mapping.hpp:23:0,
                 from /usr/include/boost/log/sinks/syslog_backend.hpp:32,
                 from /usr/include/boost/log/sinks.hpp:34,
                 from /home/wkearn/local/src/global-gradient-based-groundwater-model/tests/Unit/Model/../../../src/Model/../Misc/../Logging/Sinks.hpp:20,
                 from /home/wkearn/local/src/global-gradient-based-groundwater-model/tests/Unit/Model/../../../src/Model/../Misc/../Logging/Logging.hpp:17,
                 from /home/wkearn/local/src/global-gradient-based-groundwater-model/tests/Unit/Model/../../../src/Model/../Misc/Helpers.hpp:26,
                 from /home/wkearn/local/src/global-gradient-based-groundwater-model/tests/Unit/Model/../../../src/Model/ExternalFlows.hpp:28,
                 from /home/wkearn/local/src/global-gradient-based-groundwater-model/tests/Unit/Model/../../../src/Model/Node.hpp:34,
                 from /home/wkearn/local/src/global-gradient-based-groundwater-model/tests/Unit/Model/Node__TEST.hpp:2,
                 from /home/wkearn/local/src/global-gradient-based-groundwater-model/tests/Unit/UnitTests.hpp:2,
                 from /home/wkearn/local/src/global-gradient-based-groundwater-model/tests/testrunner.cpp:2:
/usr/include/boost/log/detail/tagged_integer.hpp: In function ‘bool boost::log::v2_mt_posix::aux::operator<(const boost::log::v2_mt_posix::aux::tagged_integer<IntT, TagT>&, const boost::log::v2_mt_posix::aux::tagged_integer<IntT, TagT>&)’:
/usr/include/boost/log/detail/tagged_integer.hpp:96:1: error: parse error in template argument list
 BOOST_LOG_TAGGED_INTEGER_OP(<)
 ^
CMakeFiles/runUnitTests.dir/build.make:254: recipe for target 'CMakeFiles/runUnitTests.dir/tests/testrunner.cpp.o' failed
make[2]: *** [CMakeFiles/runUnitTests.dir/tests/testrunner.cpp.o] Error 1
CMakeFiles/Makefile2:104: recipe for target 'CMakeFiles/runUnitTests.dir/all' failed
make[1]: *** [CMakeFiles/runUnitTests.dir/all] Error 2
Makefile:94: recipe for target 'all' failed
make: *** [all] Error 2

[Kevin-Mattheus-Moerman]

@wkearn 👏 Thanks for the detailed review! @rreinecke can you work on these comments?

[rreinecke]

@wkearn Thank you very much for your very helpful feedback! @Kevin-Mattheus-Moerman yes I will approach the mentioned issues in the next couple days.

[Kevin-Mattheus-Moerman]

@whedon generate pdf

[whedon]

Attempting PDF compilation. Reticulating splines etc...

[whedon]

[--> Check article proof :page_facing_up: <--](https://github.com/openjournals/joss-papers/blob/joss.00548/joss.00548/10.21105.joss.00548.pdf)

[arfon]

@whedon generate pdf

[whedon]

Attempting PDF compilation. Reticulating splines etc...

[whedon]

--> Check article proof 📄 <--

[Kevin-Mattheus-Moerman]

@rreinecke sorry for spamming this thread, we were testing a new way to present the link to a paper.

[rreinecke]

@wkearn concerning the libboost error. I can reproduce it with version 1.58 but not with 1.54 and I have currently no idea on what the issue actually is. Would you be so kind and open it as an issue that the build fails with that specific version and if you got the time run it down with the boost mailing list?

I've added the gtest framework now to the project build - no more need for a manual install

[rreinecke]

@wkearn

Addressed issues:
(R1): Governing equations: I've added additional descriptions concerning the equations. In general they are very similar to MODFLOW why I felt reluctant to repeat their documentation effort. I've added a link to the documentation in the description text
(R2): The simple model example: I've extended the simple model explanation - hope it is now more streamlined and helps as a basis for the user to construct his/her own model. Also added description of main model configuration parameters.
(R3): A more complex example: To be honest I've pondered on this one quite a while. Of course I have a more complex model, which is my global groundwater model, and I will link the publication as soon as it is published - but for now I probably don't have the time to provide a well documented complex example. I've added a description on the coupling process and a note that developers are welcome to contact me on coupling efforts.
(R4): Installation: As noted in the post above - 1) gtest is now shipped with the code 2) the libboost bug is reproducible with a specific version, but still a mystery to me.

Cheers
Robert.

[wkearn]

@rreinecke This looks great!

I am happy with a link to Modflow for documentation. As you say, their documentation is a lot more comprehensive, and if G³M implements basically the same equations, there is no point in duplicating that.

The simple model walkthrough is now more clear. I would encourage you to keep updating the documentation as you and others develop more applications with G³M, including your global groundwater model. Looking forward to seeing that publication when it comes out!

The installation works now, with the exception of the libboost bug. I haven't had time yet to track this down, but I will let you know if I hear anything about it.

@Kevin-Mattheus-Moerman I'm happy to recommend this for acceptance.

[Kevin-Mattheus-Moerman]

Thanks @wkearn! 🤖 Are you done with the tickboxes at the top of this issue?

@rreinecke can you make a DOI to the archived version of the reviewed software available? This is needed to process acceptance of this submission.

[wkearn]

I am now

[rreinecke]

@wkearn Thank you very much for your helpful feedback!
I'm very much looking forward to share the global model results with you and the research community.

@Kevin-Mattheus-Moerman https://zenodo.org/badge/latestdoi/109667597

[Kevin-Mattheus-Moerman]

@whedon generate pdf

[whedon]

Attempting PDF compilation. Reticulating splines etc...

[whedon]

--> Check article proof 📄 <--

[Kevin-Mattheus-Moerman]

@arfon we are good to proceed with acceptance of this paper.

[arfon]

@whedon set 10.5281/zenodo.1175540 as archive

[whedon]

OK. 10.5281/zenodo.1175540 is the archive.

[arfon]

@wkearn - many thanks for your review and to @Kevin-Mattheus-Moerman for editing this submission ✨

@rreinecke - your paper is now accepted into JOSS and your DOI is https://doi.org/10.21105/joss.00548 ⚡️:rocket: :boom:

[whedon]

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

If you would like to include a link to your paper from your README use the following code snippet:

[![DOI](http://joss.theoj.org/papers/10.21105/joss.00548/status.svg)](https://doi.org/10.21105/joss.00548)

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: http://joss.theoj.org/reviewer-signup.html
• Making a small donation to support our running costs here: https://www.flipcause.com/secure/cause_pdetails/Mjk3Nzk=