search

openjournals / joss-reviews

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

[REVIEW]: baseflow: a MATLAB and GNU Octave package for baseflow recession analysis #5492

Closed editorialbot closed 8 months ago

editorialbot commented 1 year ago

Submitting author: !--author-handle-->@mgcooper<!--end-author-handle-- (Matthew Cooper) Repository: https://github.com/mgcooper/baseflow Branch with paper.md (empty if default branch): joss Version: v1.0.0 Editor: !--editor-->@elbeejay<!--end-editor-- Reviewers: @alessandroamaranto, @tianydong Archive: 10.5281/zenodo.8401301

Status

status

Status badge code:

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

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

@deboraydo & @aymnassar & @alessandroamaranto, 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 @elbeejay 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 @alessandroamaranto

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.21 s (1264.4 files/s, 199617.5 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
HTML                            86           1914           1208          16788
MATLAB                         167           3142           8204           9454
JSON                             4              4              0            768
PHP                              1             14             26            289
Markdown                         5             92              0            251
TeX                              1             17              0            236
XML                              4             12             82             98
CSS                              1             13              0             77
YAML                             2             11             41             43
-------------------------------------------------------------------------------
SUM:                           271           5219           9561          28004
-------------------------------------------------------------------------------

gitinspector failed to run statistical information for the repository
editorialbot commented 1 year ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1016/j.cageo.2016.10.005 is OK
- 10.1029/WR013i003p00637 is OK
- 10.1029/97WR03068 is OK
- 10.1137/070710111 is OK
- 10.1029/2018WR022816 is OK
- 10.1029/2022WR033154 is OK
- 10.5194/hess-21-65-2017 is OK
- 10.1016/j.envsoft.2021.104983 is OK
- 10.1029/WR004i005p00973 is OK
- 10.1029/2008WR007392 is OK
- 10.5194/hess-24-1159-2020 is OK
- 10.1029/2008WR006912 is OK
- 10.1016/j.advwatres.2017.07.013 is OK
- 10.1029/2005WR004241 is OK
- 10.1029/2006WR005080 is OK
- 10.1002/wrcr.20407 is OK

MISSING DOIs

- None

INVALID DOIs

- None
editorialbot commented 1 year ago

Wordcount for paper.md is 1212

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:

elbeejay commented 1 year ago

Hi @deboraydo, @aymnassar, and @alessandroamaranto,

This is the official "review" issue. @mgcooper has asked that you review the content in the joss branch of the baseflow repository, so please make your comments accordingly. Instructions for creating your reviewer checklist and conducting the review should be in the top comment of this issue, but please do not hesitate to reach out to me with any questions or to ask for any clarification.

Ideally we'd like to ask that you complete your reviews within 6 weeks, and I will set up reminder so the bot prods us all in 3 weeks.

Thanks, Jay

elbeejay commented 1 year ago

@editorialbot remind @deboraydo in three weeks

editorialbot commented 1 year ago

Reminder set for @deboraydo in three weeks

elbeejay commented 1 year ago

@editorialbot remind @aymnassar in three weeks

editorialbot commented 1 year ago

Reminder set for @aymnassar in three weeks

elbeejay commented 1 year ago

@editorialbot remind @alessandroamaranto in three weeks

editorialbot commented 1 year ago

Reminder set for @alessandroamaranto in three weeks

mgcooper commented 1 year ago

@deboraydo @aymnassar @alessandroamaranto @elbeejay I aim to push the Octave-compatibility fixes tomorrow afternoon. If anyone is reviewing the toolbox exclusively in Octave, please let me know if this is an inconvenience and I will push the update asap. Thank you, Matt

alessandroamaranto commented 1 year ago

Review checklist for @alessandroamaranto

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

editorialbot commented 1 year ago

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

editorialbot commented 1 year ago

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

editorialbot commented 1 year ago

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

elbeejay commented 1 year ago

Hi @deboraydo, @aymnassar, @alessandroamaranto,

I just wanted to check in here to remind you all about this review. Feel free to reach out if you have any questions about the JOSS process.

Jay

elbeejay commented 1 year ago

@deboraydo and @aymnassar,

Just checking in here, please let me know if you still anticipate being able to complete your review in the next two weeks or so or if you'll need an extension.

@alessandroamaranto,

It looks like you've checked off all of the items on your review checklist. Can you please summarize your review briefly here and let us know if you have any suggestions or recommendations for the authors? Thanks!

alessandroamaranto commented 1 year ago

I was going to do it this morning but got caught up in some meetings.

I will submit a brief summary soon (perhaps tomorrow).

Thanks

On Wed, Jun 21, 2023, 14:31 J. Hariharan @.***> wrote:

@deboraydo https://github.com/deboraydo and @aymnassar https://github.com/aymnassar,

Just checking in here, please let me know if you still anticipate being able to complete your review in the next two weeks or so or if you'll need an extension.

@alessandroamaranto https://github.com/alessandroamaranto,

It looks like you've checked off all of the items on your review checklist. Can you please summarize your review briefly here and let us know if you have any suggestions or recommendations for the authors? Thanks!

— Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/5492#issuecomment-1600748650, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADJOC4MHCABYW4U3AMJTWRDXMLSR5ANCNFSM6AAAAAAYMG7F4Y . You are receiving this because you were mentioned.Message ID: @.***>

elbeejay commented 1 year ago

@deboraydo and @aymnassar,

Just trying to follow up here and make sure this review doesn't get lost - please check in with us here when you get a chance, thanks!

alessandroamaranto commented 1 year ago

The package is easy to install, use and understand. It is well-documented and comes with a set of workflows to ease the users through the functionalities. Much appreciated, thanks.

elbeejay commented 1 year ago

@deboraydo and @aymnassar, I'm checking in here, would be great to get an update from you both on when you might be able to review this package. Please let us know and do not hesitate to reach out if you've got any questions, thanks

elbeejay commented 12 months ago

@deboraydo and @aymnassar, please let me know if you both are still planning on completing this review; it has been some time since we have heard from either of you. If you are unable to review, please tell us so we can begin the process of contacting and finding alternative reviewers - thanks.

cc @kthyng

elbeejay commented 11 months ago

@mgcooper we seem to have lost contact with two of our reviewers. Given the positive review from @alessandroamaranto I am thinking we will look for a single replacement reviewer to replace them in the hopes of keeping this process from dragging out too long. Thank you for your patience.

elbeejay commented 11 months ago

@mgcooper, @tianydong has agreed to step in and review your submission. He's out of the office through Aug. 6, but anticipates being able to conduct his review in the third week of August.

Thanks @mgcooper for your patience, and big thanks to @tianydong for stepping in to review!

elbeejay commented 11 months ago

@tianydong do you have any updates on this review? Thanks!

tianydong commented 11 months ago

Overall the package is easy to install, the codes are well-commented, and demo 1 runs fine. But I am getting some errors on the other demos, for example: image

Maybe a path thing? Also, I am using MATLAB 2022b, but I wonder if that's a problem. Also, there's a conflict between the .m and .mlx files for the linear theory demos. image does this mean the user suppose to stick with LiveScript? Thanks again for having me as part of the review. I will provide some comments on the paper shortly

tianydong commented 11 months ago

I made some minor comments, but overall the paper is well-written, and the figure is nicely made. Good luck! paper_TDedits.pdf

elbeejay commented 10 months ago

Thanks for the review @tianydong - please look over the marked up paper @mgcooper and make revisions and provide responses for the comments as appropriate. The error above should also be addressed, thanks!

mgcooper commented 10 months ago

@elbeejay @tianydong @alessandroamaranto

Thank you kindly for the review, @tianydong and @alessandroamaranto! I will formally respond to the edits and make the needed code revisions to resolve the errors, but wanted to quickly explain them. The first one occurs because the input parser allows the argument T to be either double (for datenum compatibility) or datetime. But then I have a second check (the one throwing the error, on line 180) which is requiring T to be double. So I will just add 'datetime' to that validation function. This is a newer change I pushed recently, so it did not affect @alessandroamaranto version.

The second error is because Matlab does not allow m-files (.m) to have the same name as live scripts (.mlx). To work around this, I used "demo" for the .m files and "example" for the .mlx. For example, "bfra_demo_1.m" is identical to "bfra_example_1.mlx" in terms of the content, but the former is a traditional .m file and the latter is in the newer live script format. I just completely overlooked this name shadowing for the two "theory" examples. So I'll rename them for the revisions. You could also change the name to anything other than the current one and it should work (as long as the new name doesn't shadow some other file) if you want to try running it.

I'll get these fixes pushed asap. Thanks again for the reviews and many thanks @elbeejay for managing the submission. Matt

mgcooper commented 10 months ago

@elbeejay @tianydong @alessandroamaranto @hydrotian

Revisions are complete and pushed to the joss branch (also main). Updates include:

Some important changes were made to the repository structure:

From a user-perspective, the reorganization only affects the tests. Instead of typing bfra.test.runtests, just type runtests. Overall, the new structure aligns with standard software development practices. I think it is important for the release associated with this paper.

Please see a point by point reply to reviewer comments below.

@tianydong "Overall the package is easy to install, the codes are well-commented, and demo 1 runs fine. But I am getting some errors on the other demos, for example: "

@tianydong "Maybe a path thing? Also, I am using MATLAB 2022b, but I wonder if that's a problem."

@tianydong "Also, there's a conflict between the .m and .mlx files for the linear theory demos."

We believe this satisfies all requests from reviewers. Please let us know if any issues are encountered running the software. Many thanks for the review.

Matt Cooper Tian Zhou

elbeejay commented 10 months ago

Thank you for the revisions, @tianydong please weigh in on the responses made to your review comments when you get a chance.

tianydong commented 10 months ago

All looks good! Thank you. @elbeejay

@mgcooper did you update the paper itself, by the way?

mgcooper commented 10 months ago

@tianydong We did update the paper, our apologies for omitting that from the response. Specifically, to address each of your comments:

  1. "maybe add one or two examples of aquifer properties, like K or porosity, here"

As suggested, we added examples of aquifer properties to the abstract.

  1. "I'd suggest adding one sentence here to quickly define baseflow."

As suggested, we added a sentence to the first paragraph to define baseflow. We also broke the first paragraph into two. The first paragraph includes the baseflow definition, as requested, and a statement about why baseflow matters. The second paragraph addresses comment #3, specifically "how exactly recession curve can be used to infer ... aquifer properties".

  1. "If space permit, I'd also suggest elaborate how exactly recession curve can be used to infer geological structures, particularly to the aquifer properties that baseflow is able to infer (mentioned later in the paper)"

We removed the phrase "geologic structures" to avoid confusion among those with a groundwater hydrology background. We also removed this phrase from the abstract. To answer your question, the geologic structures themselves cannot be inferred, rather the theory is based on an assumption that the aquifer is unconfined. To address this, we emphasize "unconfined aquifer properties" throughout the paper, and removed "geologic structures".

To address the other side of your comment: "how exactly recession curve can be used to infer ... aquifer properties", see our first two answers above. Additionally, we added a sentence to the first paragraph of the "State of the field" section that directly addresses this question: "Several well-known solutions to the one-dimensional lateral groundwater flow equation for unconfined aquifers can be written in the same form as [the recession equation]" and this forms the basis for estimating aquifer properties from the parameters obtained by curve-fitting recession sequences.

We also added a sentence immediately before "State of the field" that mentions the two theory-focused notebooks, which also provide more specific information about how recession curves can be used to infer aquifer properties.

  1. "I'd suggest explain each panel in the figure caption" As requested, we added an explanation of each panel to the figure caption.

Finally, we made various edits to improve the writing quality and/or grammar. Please see the compiled pdf in the paper/ folder at the top-level of the repo.

Thank you,

Matt Cooper Tian Zhou

elbeejay commented 10 months ago

Great, thanks all. I'm not going to be able to get to this until next week but will do a final check and get back to you @mgcooper with next steps then.

elbeejay commented 10 months ago

@editorialbot check references

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

OK DOIs

- 10.1016/j.cageo.2016.10.005 is OK
- 10.1029/WR013i003p00637 is OK
- 10.1029/97WR03068 is OK
- 10.1137/070710111 is OK
- 10.1029/2018WR022816 is OK
- 10.1029/2022WR033154 is OK
- 10.5194/hess-21-65-2017 is OK
- 10.1016/j.envsoft.2021.104983 is OK
- 10.1029/WR004i005p00973 is OK
- 10.1029/2008WR007392 is OK
- 10.5194/hess-24-1159-2020 is OK
- 10.1029/2008WR006912 is OK
- 10.1016/j.advwatres.2017.07.013 is OK
- 10.1029/2005WR004241 is OK
- 10.1016/j.advwatres.2005.03.019 is OK
- 10.1029/2006WR005080 is OK
- 10.1029/92WR02087 is OK
- 10.1002/wrcr.20407 is OK

MISSING DOIs

- None

INVALID DOIs

- None
elbeejay commented 10 months ago

@editorialbot generate pdf

editorialbot commented 10 months ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

elbeejay commented 10 months ago

It looks good to me, thanks again to @alessandroamaranto and @tianydong for providing their reviews.

@mgcooper I'm going to generate a post-review checklist now that has a few last things for you to do before I can recommend this submission for publication.

elbeejay commented 10 months ago

Post-Review Checklist for Editor and Authors

Additional Author Tasks After Review is Complete

Editor Tasks Prior to Acceptance

elbeejay commented 10 months ago

@mgcooper if you could notify me when you've done the author items on the above list that'd be great (as it's my comment so you won't be able to edit that checklist directly). Then I'll go ahead and go through the editor part of the checklist and we'll be ready to recommend this paper for publication. Thanks!

elbeejay commented 9 months ago

@mgcooper what's your estimated timeline for taking care of these last few items? They should be pretty quick, they exist to ensure the metadata associated with the JOSS publication is up to date.

mgcooper commented 9 months ago

@elbeejay My apologies for the delay. I found some Octave incompatibilities that I wanted to address for the release. I will complete these last few items today! Thank you for your attention, Matt

mgcooper commented 9 months ago

@elbeejay

I completed the check list. Below are my replies to action items:

Release version is v1.0.0. I made a v1.0.0-joss-branch release which used the joss branch as the target and another on main branch. Feel free to use whichever you prefer, but I wanted a v1.0.0 release to point to main for future clarity.

Please let me know if anything remains. Thank you, Matt

elbeejay commented 9 months ago

Hi @mgcooper - I need the title of the zenodo entry to match the title of the paper, if you could do that it'd be great.

elbeejay commented 9 months ago

@editorialbot set v1.0.0 as version

editorialbot commented 9 months ago

Done! version is now v1.0.0

elbeejay commented 9 months ago

@mgcooper is there a reason the license is in a file called COPYING? as opposed to one called LICENSE? I think the latter would be our preference as it is more standard.