openjournals / joss-reviews

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

[REVIEW]: pyHMT2D: Python-based two-dimensional hydraulic modeling tools #3332

Closed whedon closed 3 years ago

whedon commented 3 years ago

Submitting author: @psu-efd (Xiaofeng Liu) Repository: https://github.com/psu-efd/pyHMT2D Version: v1.0.1 Editor: @kthyng Reviewers: @jmp75, @mdbartos, @dbuscombe-usgs Archive: Pending

:warning: JOSS reduced service mode :warning:

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

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

@jmp75 & @mdbartos & @dbuscombe-usgs, 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 @jmp75

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Review checklist for @mdbartos

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Review checklist for @dbuscombe-usgs

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

whedon commented 3 years ago

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

:warning: JOSS reduced service mode :warning:

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.

:star: Important :star:

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 commented 3 years ago
Software report (experimental):

github.com/AlDanial/cloc v 1.88  T=0.29 s (322.9 files/s, 51755.4 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Python                          56           3316           3564           4800
JSON                            11              0              0           1064
SVG                              1              1              1            797
reStructuredText                16            260             95            524
TeX                              1             24              0            169
Bourne Shell                     1             32             10            141
Markdown                         4             62              0            138
YAML                             1              1              4             18
make                             1              4              8             10
HTML                             1              5              0              9
INI                              1              0              0              9
-------------------------------------------------------------------------------
SUM:                            94           3705           3682           7679
-------------------------------------------------------------------------------

Statistical information for the repository '3487dd725c6a97e1e4b938ad' was
gathered on 2021/06/04.
The following historical commit information, by author, was found:

Author                     Commits    Insertions      Deletions    % of changes
psuliuxf                        49         14883           3022          100.00

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
psuliuxf                  11861           79.7          1.5               16.98
kthyng commented 3 years ago

@jmp75, @mdbartos here is where the review takes place! Please let me know if you have any questions.

jmp75 commented 3 years ago

@whedon generate pdf

kthyng commented 3 years ago

The paper compilation has been having issues, but Arfon compiled it and for now it can be found here

jmp75 commented 3 years ago

I am about halfway through the checklist. I note that I cannot tick the boxes above, as the process suggests, and the link to the invitation is not active or not anymore.

danielskatz commented 3 years ago

@whedon re-invite @jmp75 as reviewer

This 👇 should fix the problem

whedon commented 3 years ago

OK, the reviewer has been re-invited.

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

whedon commented 3 years ago

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

whedon commented 3 years ago

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

kthyng commented 3 years ago

@mdbartos When might you be able to work on your review?

@jmp75 are you able to continue with your review?

jmp75 commented 3 years ago

@kthyng yes; I was distracted by some end of financial year busy work the past few weeks; I can complete the checklist in the coming days.

mdbartos commented 3 years ago

Same; just getting back from some summer travel and will complete my checklist soon.

kthyng commented 3 years ago

Thanks @jmp75 and @mdbartos! I know this is a really busy time of year.

mdbartos commented 3 years ago

@whedon re-invite @mdbartos as reviewer

whedon commented 3 years ago

I'm sorry @mdbartos, I'm afraid I can't do that. That's something only editors are allowed to do.

mdbartos commented 3 years ago

Can I get a re-invite? I can't currently access the checklist.

kthyng commented 3 years ago

@whedon re-invite @mdbartos as reviewer

whedon commented 3 years ago

The reviewer already has a pending invite.

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

kthyng commented 3 years ago

Will this invitation work for you?

mdbartos commented 3 years ago

Greetings,

Thanks for your help. Unfortunately, the link doesn't seem to work for me.

I've completed all the checklist items that I can complete. Unfortunately, I am not sure that I will be able to complete all aspects of the review at this time. The package is Windows-only (https://github.com/psu-efd/pyHMT2D/issues/4), and also requires access to external software that is only supported in Windows.

Currently, I only have access to mac and linux machines, and thus can't install/test the software. I'm planning to purchase a couple of Windows machines in the near future for another project, but I'm not exactly sure about the timeline.

Perhaps it might be possible to install the package on a windows instance in the cloud?

Let me know how you'd like to proceed.

Thanks, MDB

psu-efd commented 3 years ago

Hi Matt,

Thanks for reviewing this work. How about virtual machine?

Xiaofeng Liu

From: Matt Bartos @.> Sent: Wednesday, July 21, 2021 12:47 AM To: openjournals/joss-reviews @.> Cc: Dr. Xiaofeng Liu, Environmental Fluid Dynamics Group, Penn State University @.>; Mention @.> Subject: Re: [openjournals/joss-reviews] [REVIEW]: pyHMT2D: Python-based two-dimensional hydraulic modeling tools (#3332)

Greetings,

Thanks for your help. Unfortunately, the link doesn't seem to work for me.

I've completed all the checklist items that I can complete. Unfortunately, I am not sure that I will be able to complete all aspects of the review at this time. The package is Windows-only (psu-efd/pyHMT2D#4 https://github.com/psu-efd/pyHMT2D/issues/4 ), and also requires access to external software that is only supported in Windows.

Currently, I only have access to mac and linux machines, and thus can't install/test the software. I'm planning to purchase a couple of Windows machines in the near future for another project, but I'm not exactly sure about the timeline.

Perhaps it might be possible to install the package on a windows instance in the cloud?

Let me know how you'd like to proceed.

Thanks, MDB

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/3332#issuecomment-883884641 , or unsubscribe https://github.com/notifications/unsubscribe-auth/ADBFJD2YDQTNZY2GU6CESE3TYZGODANCNFSM46DSIL5A .

kthyng commented 3 years ago

@mdbartos Do you have multiple github accounts? If so make sure you are signed into the one associated with this review when clicking on the link and working on the review. If there is still an issue I'll ask for help.

@mdbartos and @psu-efd: Will a virtual machine approach work? Or something like Binder? The limitation of this being able to run in Windows only should be clear in the readme as well as the docs.

psu-efd commented 3 years ago

@mdbartos Do you have multiple github accounts? If so make sure you are signed into the one associated with this review when clicking on the link and working on the review. If there is still an issue I'll ask for help.

@mdbartos and @psu-efd: Will a virtual machine approach work? Or something like Binder? The limitation of this being able to run in Windows only should be clear in the readme as well as the docs.

I think a virtual machine should work. Binder will not work. This package is developed to help run 2D hydraulics models, which currently support U.S. Army Corps of Engineer's HEC-RAS and U.S. Bureau of Reclamation's SRH-2D. Both only run on Windows because they are used by engineers and practitioners.

I will highlight in the README and the User Manual that currently only Windows is supported (this should be apparent for users of this Python Package though).

kthyng commented 3 years ago

The user manual currently specifically says that it works with Mac and Linux, and of course the point of a user manual is to help new users figure out how to use your software including whether they should be using it at all. Same for the JOSS paper.

I think it is a valid argument that engineers tend to use Windows, but certainly that doesn't mean the software should by definition only run on Windows if engineers use it.

@psu-efd Do you have everything set up for a virtual machine so that a user can easily set it up and run it?

psu-efd commented 3 years ago

The user manual currently specifically says that it works with Mac and Linux, and of course the point of a user manual is to help new users figure out how to use your software including whether they should be using it at all. Same for the JOSS paper.

That is a good point. I have temporally changed the README to point out that if the user wants to run 2D models on Windows, then they have to install and use pyHMT2D on Windows. There are other 2D models (currently not supported) which can run on Linux and Mac. pyHMT2D can be used on Linux and Mac because it is a python library and the python code itself can be run on any OS. That is why on Page 6, Section 2.1.1 I pointed out that "We use Windows operating system as an example. pyHMT2D can be used in Linux and Mac OS".

I think it is a valid argument that engineers tend to use Windows, but certainly that doesn't mean the software should by definition only run on Windows if engineers use it.

@psu-efd Do you have everything set up for a virtual machine so that a user can easily set it up and run it?

Virtual machine should work. If anyone wants to try VM, Windows should be installed first and then HEC-RAS and SRH-2D. The rest is identical to a dedicated Windows machine. Another option is to use Cloud service (select a Windows instance).

I hope my answer is to your satisfaction. Thanks.

kthyng commented 3 years ago

@psu-efd For the review process, the reviewers need to actually run your software. So, either you need to make it really easy for @mdbartos to do this with a virtual machine setup or another approach (by giving clear step-by-step instructions, etc), or we need to add another reviewer who can run the windows-based software. Please remember that the reviewers are volunteering their time to improve your software through this review process, so we need to make the mechanisms for that as easy and fast as we can. Additionally, you may have future users who would want to use your software but have the same problem.

mdbartos commented 3 years ago

Another solution could be to include pre-compiled binaries for different platforms, like PySWMM does with the EPA stormwater management model (https://github.com/OpenWaterAnalytics/pyswmm).

Not sure about whether USBR or USACE provide access to source code though.

psu-efd commented 3 years ago

@psu-efd For the review process, the reviewers need to actually run your software. So, either you need to make it really easy for @mdbartos to do this with a virtual machine setup or another approach (by giving clear step-by-step instructions, etc), or we need to add another reviewer who can run the windows-based software. Please remember that the reviewers are volunteering their time to improve your software through this review process, so we need to make the mechanisms for that as easy and fast as we can. Additionally, you may have future users who would want to use your software but have the same problem.

I totally understand this. @mdbartos You don't have access to Windows computer at all? If that is the case, I think the last resort is to use cloud computing service such as AWS. They provide free tier Windows instance.

@kthyng: I know the review process sometime can get unexpected issues. If @mdbartos can't use Windows, we may have to find another reviewer.

psu-efd commented 3 years ago

Another solution could be to include pre-compiled binaries for different platforms, like PySWMM does with the EPA stormwater management model (https://github.com/OpenWaterAnalytics/pyswmm).

Not sure about whether USBR or USACE provide access to source code though.

pyHMT2D is something similar to PySWMM. But pyHMT2D itself does not contain the model code. Unfortunately both HEC-RAS and SRH-2D are closed source. So I can't compile them into a library as part of pyHMT2D.

mdbartos commented 3 years ago

Unfortunately, I do not have access to a physical Windows machine right now, although I am in the process of ordering one through my department. I estimate that it should take a week or three.

If you can provide documentation describing how to install/test pyHMT2D on a cloud instance, that could also work.

If you decide to go with another reviewer, that is also OK.

psu-efd commented 3 years ago

Again, thanks for your time and effort.

For the use in cloud computing, I am not sure whether you have used AWS, Azure, or Google Cloud. Basically, you need to start a Windows instance (try the free ones) and then remote desktop to the instance. After that, it is the same as your own Windows computer. You can install whatever you like.

From: Matt Bartos @.> Sent: Wednesday, July 28, 2021 1:44 AM To: openjournals/joss-reviews @.> Cc: Dr. Xiaofeng Liu, Environmental Fluid Dynamics Group, Penn State University @.>; Mention @.> Subject: Re: [openjournals/joss-reviews] [REVIEW]: pyHMT2D: Python-based two-dimensional hydraulic modeling tools (#3332)

Unfortunately, I do not have access to a physical Windows machine right now, although I am in the process of ordering one through my department. I estimate that it should take a week or three.

If you can provide documentation describing how to install/test pyHMT2D on a cloud instance, that could also work.

If you decide to go with another reviewer, that is also OK.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/3332#issuecomment-888026940 , or unsubscribe https://github.com/notifications/unsubscribe-auth/ADBFJD4T4LREBWO5G4Z7LVTTZ6KJPANCNFSM46DSIL5A .

kthyng commented 3 years ago

Ok I will bring in another review who uses Windows in order to proceed with this review.

@AtrCheema are you interested in reviewing this JOSS software? The review is already underway but we need another reviewer who has access to Windows to complete the process. Would you be able to fill this role? Thanks.

kthyng commented 3 years ago

@dbuscombe-usgs are you interested in reviewing this JOSS software? The review is already underway but we need another reviewer who has access to Windows to complete the process. Would you be able to fill this role? Thanks.

dbuscombe-usgs commented 3 years ago

Yes, I have access to Windows and some prior experience with numerical flow modeling, so I could do this! (I might need 2 weeks)

kthyng commented 3 years ago

@dbuscombe-usgs Wonderful! Thank you. I will add you as a reviewer soon.

kthyng commented 3 years ago

@mdbartos You mentioned above that you were able to complete some checkboxes in the review process before needing access to Windows, but none of the boxes are checked above. Were you able to complete some? If so, I would like to preserve that fact by having your review stay in place with the boxes checked that you were able to check along with any notes. If you really weren't able to do much of the review, I can just remove you as a reviewer and remove you from this issue. What do you think?

mdbartos commented 3 years ago

Hi @kthyng, I was able to complete everything except for the functionality component. I cannot check the boxes, as I wasn't able to get the reviewer invitation to work.

Apologies for not being able to complete the review. Although I could potentially test the software on an AWS instance, the process of installing and setting up the necessary software in this environment is not necessarily straightforward, and I would need step-by-step instructions from someone familiar with the software.

kthyng commented 3 years ago

@whedon add @dbuscombe-usgs as reviewer

whedon commented 3 years ago

OK, @dbuscombe-usgs is now a reviewer

kthyng commented 3 years ago

Ok, I think I have everything squared away but please let me know if anything doesn't look correct.

@mdbartos You remain a reviewer, but I will note that not all of the boxes are checked off because you were unable to fully complete the reviewer, not having a Windows machine. Also because you were never able to edit the check boxes, please check the boxes that I checked off to make sure you agree with what is and is not checked.

@dbuscombe-usgs I have added you as a reviewer. I think everything should now be in order for you to check off your own check boxes and pursue the review. Thank you for stepping in to help here!

dbuscombe-usgs commented 3 years ago

My invitation has expired so I cant check boxes.

Also, this was confusing before - is the expectation that the boxes on the github site itself are editable by me? some of them are already checked (sorry, the boxes thing confused me before and continues to confuse me)

danielskatz commented 3 years ago

@whedon re-invite dbuscombe-usgs as review

this should fix the permission to check boxes, but I'm unsure about the previously checked boxes, so will let @kthyng answer that part

whedon commented 3 years ago

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

@whedon commands
danielskatz commented 3 years ago

@whedon re-invite dbuscombe-usgs as reviewer

whedon commented 3 years ago

OK, the reviewer has been re-invited.

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

dbuscombe-usgs commented 3 years ago

Ok, that worked. I can now check buttons. It will take me a little longer to figure out IF I can complete this review, because it relies on installation of third party software (that isn't open source and that I havent used before). Given that, its a larger task than I previously supposed. Also, the provided instructions immediately fail because it requires knowing how to make a calibration.json file. I see examples on the github repo that I will try to make work. I'll report back whether or not I am actually capable of reviewing this. I initially thought it was a pure python package

dbuscombe-usgs commented 3 years ago

Also, it now appears I have been subscribed to notifications from JOSS articles that I am not reviewing ... is there a way you can disable that, while retaining my access to this repo for the checklist?

kthyng commented 3 years ago

Hi @dbuscombe-usgs — sorry not to properly represent this package for reviewing. Please let me know what you determine about whether you can actually review it or not.

I think the boxes must be checked because I copied them from another reviewer. I will go uncheck all of your boxes.

As for all the JOSS notifications — check out the comment above to see how to unwatch the JOSS review repo. We can't unwatch on your behalf, unfortunately.

dbuscombe-usgs commented 3 years ago

Got it - thanks!