[REVIEW]: r2ogs6: An R wrapper of the OpenGeoSys 6 Multiphysics Simulator

editorialbot commented 1 year ago

Submitting author: !--author-handle-->@joboog@kthyng<!--end-editor-- Reviewers: @waternumbers, @ldecicco-USGS Archive: Pending

Status

Status badge code:

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

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

@ldecicco-USGS & @waternumbers, 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 @jsta 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 @waternumbers

📝 Checklist for @ldecicco-USGS

joboog commented 5 months ago

Package feedback

At this point I'm still unable to install the proper version of vtk (8.2). Until the r2ogs6 package can handle vtk 9.3.0 which is the version that is linked from the doc (https://pypi.org/project/vtk/), I don't think I can review the package functionality. All of the functions that are used to create the ogs6_obj work perfectly. I therefore assume ogs6_run_simulation and ogs6_read_output_files would work - but they require the older vtk library.

Since this package is for R users, it will be important to include detailed instructions on how to install the mandatory Python libraries. The reason I would choose this package over the Python ogs6py package is because I'm much more comfortable in R. The reason I'm more comfortable in R is because installing things in Python gives me a headache 😂

##########

Paper feedback:

The paper nicely outlines why someone would be interested in this package, and gives a nice example of how to use it. I would recommend cleaning up the final paragraph.

"Please, check the following package vignettes for more information: a) a further guide on how to create ensemble runs (link), b) a tutorial to set up a single simulation of a hydro-mechanics benchmark with the package functions (link), and c) a guide how to start to further develop the package (link)."

Would it be possible to create a GitLab page using pkgdown, and link to a rendered version of the Rmd? I'd also suggest removing the "Please, " to just read:

"Check the following package vignettes...."

I updated the package and the paper accordingly.

@Package Feedback

Added support to use VTK v9.0.3
There is now also a function to install OpenGeoSys 6 as python module (install_ogs())
The section r2ogs6-installation was revised accordingly.

@Paper Feedback

The paragraph was revised to

"Check the following package vignettes for more information:
- a guide to set up a single simulation of a hydro-mechanics benchmark (link)
- a guide to create ensemble runs (link)
- a development guide (link)"
Gitlab page with vignettes was created link

kthyng commented 5 months ago

@ldecicco-USGS It looks like this would be a good time for you to check back to see if you can finish your review, if possible.

ldecicco-USGS commented 4 months ago

The reticulate instructions are great! As an R user, that seemed a lot more streamlined for setting up the environments.

That being said, I'm still seeing:

Error in ogs6_read_output_files(ogs6_obj = ogs6_obj) : 
  Output file not written out correctly.
                    Unable to import *.pvd

When I look at the r2ogs6 CI tests, the pipeline is failing in the same way:

https://gitlab.opengeosys.org/ogs/tools/r2ogs6/-/jobs/434727

kthyng commented 4 months ago

@joboog Can you address this comment?

joboog commented 3 months ago

@ldecicco-USGS sometimes reticulate is not pointing to the created virtual python environment where the ogs package is installed. Then the simulations are actually not run. I did some bugfixes (on master branch). Please try again using the updated instructions in the README of the actual master branch.

kthyng commented 2 months ago

@ldecicco-USGS Can you try with the updated instructions please?

kthyng commented 1 month ago

Hi @ldecicco-USGS! I know it's hard having this draw out over a long period of time like this, plus you might be traveling like I have been lately. When might you be able to get back to this? Thanks.

ldecicco-USGS commented 1 month ago

Ack sorry! I'll look at it now!

ldecicco-USGS commented 1 month ago

IT WORKED! 🎉 The installation on a Windows computer using reticulate worked following the instructions on the README. Thank you for adding those! As a Windows R user with limited admin access to my own computer (which...is not uncommon for a government worker, or some industries), that is not uncommon.

The response to the original review sounds great!

kthyng commented 1 month ago

@ldecicco-USGS Where does that leave us with your review then? I see a few checkboxes unchecked — are those finished or are there outstanding issues yet?

ldecicco-USGS commented 1 month ago

Those are now good (I just edited the review to check them off) - I was able to verify the remaining examples and installation, so I think we can move on to the next phase.

kthyng commented 1 month ago

Woohoo!!! Thank you SO MUCH @ldecicco-USGS! You really saved the day... or year as the case may be.

@joboog I went back and found these issues opened by reviewer @waternumbers https://gitlab.opengeosys.org/ogs/tools/r2ogs6/-/issues/86, https://gitlab.opengeosys.org/ogs/tools/r2ogs6/-/issues/87. The first looks like you addressed it, but not the second. Can you update me on the status?

joboog commented 1 month ago

@kthyng my replies to https://gitlab.opengeosys.org/ogs/tools/r2ogs6/-/issues/86 had all been given, I am actually working on one last item from https://gitlab.opengeosys.org/ogs/tools/r2ogs6/-/issues/87.

kthyng commented 1 month ago

Excellent! Let me know when you're finished.

kthyng commented 1 week ago

@joboog How are things going?

openjournals / joss-reviews