Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[REVIEW]: UWGeodynamics: A teaching an research tool for numerical geodynamic modelling #1136

Closed
whedon opened this Issue Dec 18, 2018 · 70 comments

Comments

Projects
None yet
6 participants
@whedon
Copy link
Collaborator

whedon commented Dec 18, 2018

Submitting author: @rbeucher (Romain Beucher)
Repository: https://github.com/underworldcode/UWGeodynamics
Version: 2.7.7
Editor: @lheagy
Reviewers: @flohorovicic
Archive: 10.5281/zenodo.2636105

Status

status

Status badge code:

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

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

Reviewers, 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 @lheagy know.

Please try and complete your review in the next two weeks

Review checklist for @flohorovicic

Conflict of interest

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: 2.7.7
  • Authorship: Has the submitting author (@rbeucher) 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)?

Review checklist for @tth030

Conflict of interest

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: 2.7.7
  • Authorship: Has the submitting author (@rbeucher) 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

This comment has been minimized.

Copy link
Collaborator Author

whedon commented Dec 18, 2018

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

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
@whedon

This comment has been minimized.

Copy link
Collaborator Author

whedon commented Dec 18, 2018

Attempting PDF compilation. Reticulating splines etc...
@whedon

This comment has been minimized.

Copy link
Collaborator Author

whedon commented Dec 18, 2018

@lheagy

This comment has been minimized.

Copy link
Member

lheagy commented Dec 18, 2018

Many thanks @techas, @tth030 for being willing to review! 🎉

First of all, if you have not signed up to be a reviewer with JOSS before, please accept the invitation here: https://github.com/openjournals/joss-reviews/invitations. This will allow you to check-off the boxes above.

There is a checklist for each of you to help guide the review. If there are items that you see are missing or could be improved, please either comment here or create an issue in the target repository and reference this issue by including openjournals/joss-reviews#1136 in the text of the issue.

Please let me know if you have any questions or if I can clarify anything.

@lheagy

This comment has been minimized.

Copy link
Member

lheagy commented Jan 7, 2019

👋 Hi @techas, @tth030, I hope you had a wonderful time over the holidays! Now that the new year is starting to get going, I wanted to follow up and ask when you think you will have time to complete the review? Ideally, we would appreciate if you could do so in the next 2 weeks. Please let me know if you have any questions!

@lheagy

This comment has been minimized.

Copy link
Member

lheagy commented Jan 20, 2019

👋 Hi @techas, @tth030, just checking in on your progress with the review. Are there any questions you have about the procedure? Please don't hesitate to let me know if I can be of assistance

@tth030

This comment has been minimized.

Copy link
Collaborator

tth030 commented Jan 21, 2019

@lheagy

This comment has been minimized.

Copy link
Member

lheagy commented Jan 21, 2019

Thanks @tth030, sounds good. You can leave your review as a comment on this issues thread, or if you have specific suggestions, then please open up issues on the target repository. Then @rbeucher can address them there.

@tth030

This comment has been minimized.

Copy link
Collaborator

tth030 commented Jan 23, 2019

@lheagy

This comment has been minimized.

Copy link
Member

lheagy commented Jan 27, 2019

Hi @tth030, thanks for keeping us in the loop. Not a problem on the extension. Best of luck getting through your current projects

@tth030

This comment has been minimized.

Copy link
Collaborator

tth030 commented Feb 3, 2019

Installation: Does installation proceed as outlined in the documentation?

** NO

After many tests, I was unable to test UWGeodynamics.

LOCAL INSTALLATION
I worked under Mac Os x 10.12.6.
Underworld2 is required to run UWGeodynamics. I believe that my system is well set. I use Spack to deal with my environment. I went through the installation using python2.7.15 but ipython5.8.0 is freezing when importing underworld2...
Using Python3.6.5 I got this message ""PETSc was configured with one OpenMPI mpi.h version but now appears to be compiling using a different OpenMPI mpi.h version"" when running "scons.py" during underworld2 installation.
Unfortunately, I do not have so much time to go further other tests for a local installation.

DOCKER INSTALLATION

To be honest this was painful to have to install Docker (>> 1.8 Gb !!!!!!)...
Docker then ask for privileges on my laptop...
We then have a huge black box installation in docker >> 1-2 Gb ...
This is a crazy way only to install python packages...
--port 8888:8888 \ is not a recognized option by docker...

The installation process has to be improved. I understand that authors want simplify users life thanks to the use of a container because of underworld installation. It looks like underworld2 to be really sensitive to installation process (compiler, versions...?) and UWGeodynamics can only be run if underworld2 is installed.

I stop my review here. I apologize to JOSS and authors but I do not have enough time to go further this review.

Regards,

@lheagy

This comment has been minimized.

Copy link
Member

lheagy commented Feb 4, 2019

Thanks @tth030 for taking time to work through this. I can find someone else to step in as we move forward.

@rbeucher, I appreciate that some of the software installation steps might depend on how Underworld2 distributes software. One suggestion for simplifying things and demonstrating that the software can be installed would be to use binder: https://mybinder.org/ (which can take docker files) and provide an example notebook that uses UWGeodynamics (this could be in the same repository or a different one - however you prefer to organize things). The reviewer can then work through the installation steps you provide or use the example hosted on binder to complete the review. The docs on binder are here (https://mybinder.readthedocs.io/en/latest/). I am happy to answer any questions about getting up and running on binder.

@rbeucher

This comment has been minimized.

Copy link

rbeucher commented Feb 4, 2019

Thanks @lheagy. Sure I can set up Binder.

@rbeucher

This comment has been minimized.

Copy link

rbeucher commented Feb 4, 2019

Thanks for your time @tth030. I appreciate that the Underworld dependencies, mainly petsc and hdf can be hard to compile, especially if one is not used to do that sort of things. It indeed requires skills and I am sorry you could not get it to work. This is why we provide a Docker image. The Docker team provides detailed instructions on how to install docker. Now the image is indeed quite big. This is because it contains an OS and the full stack of dependencies required to run UWGeodynamics models, not just a python package. I wish you good luck in sorting out your busy schedule.

@rbeucher

This comment has been minimized.

Copy link

rbeucher commented Feb 5, 2019

Hi @lheagy,

I have set up a binder https://mybinder.org/v2/gh/rbeucher/UWGeodynamics-binder/master
I will add the link to the main repository.
I can suggest 2 reviewers:
Anthony Jourdon, University of Toulouse France, anthony.jourdon@get.omp.eu
Florian Wellman, Aachen University, wellmann@aices.rwth-aachen.de

Regards,
Romain Beucher

@lheagy

This comment has been minimized.

Copy link
Member

lheagy commented Feb 24, 2019

Hi @rbeucher, sorry for the delay, I haven't heard back from Anthony or Florian yet, so I am still on the hunt for reviewers. I will keep you posted.

@rbeucher

This comment has been minimized.

Copy link

rbeucher commented Feb 24, 2019

Hi @lheagy , thanks for letting me know.

@lheagy

This comment has been minimized.

Copy link
Member

lheagy commented Feb 27, 2019

@whedon assign @flohorovicic as reviewer

@whedon whedon assigned lheagy and unassigned lheagy Feb 27, 2019

@whedon

This comment has been minimized.

Copy link
Collaborator Author

whedon commented Feb 27, 2019

OK, the reviewer is @flohorovicic

@lheagy

This comment has been minimized.

Copy link
Member

lheagy commented Feb 27, 2019

Many thanks @flohorovicic for being willing to review! If you haven't done so already, please accept the invitation from: https://github.com/openjournals/joss-reviews/invitations. This will allow you to check off the tick-boxes in the main thread above. If there are items that you see are missing or could be improved, please either comment here or create an issue in the target repository and reference this issue by including openjournals/joss-reviews#1136 in the text of the issue.

The process for installing some of the dependencies is a bit involved, but @rbeucher has set up a binder where you can test-drive the software: https://mybinder.org/v2/gh/rbeucher/UWGeodynamics-binder/master

Please don't hesitate to reach out if you have any questions. Thanks!

@labarba

This comment has been minimized.

Copy link
Member

labarba commented Mar 17, 2019

Hi @flohorovicic 👋 — When will you be able to work on this review? Give us a status update, when you can. Thanks!

@flohorovicic

This comment has been minimized.

Copy link
Collaborator

flohorovicic commented Mar 17, 2019

Dear @labarba, finally on top of my pile and I will be done in the next couple of days (at latest by the end of the week)! Kind regards from Aachen, Florian.

@flohorovicic

This comment has been minimized.

Copy link
Collaborator

flohorovicic commented Mar 27, 2019

The provided software and description provides a really low entry point for the use of numerical simulations for typical applications in geodynamics. Especially the interface to Python makes using this software with all its underlying potential now very easy and I can see a wide potential for the use in teaching as well as research.

One aspect that is a bit puzzling is the separation from the underworld2 project (https://github.com/underworldcode/underworld2). When going through the examples and looking online, I commonly ended up on information referring to underworld2 - especially as it also has a Python interface and some (also very interesting) example notebooks. I ended up running several of these notebooks before realising it is a different repository... It is not directly obvious why they are two different projects.

Suggestion: extend information on the UWGeodynamics readme file, clarify difference to underworld2 and include also installation information directly there (and a link to the notebooks on binder).

Installation

The suggested installation over Docker worked without any problems. Small suggestion: add some notes on how to stop a container (and even delete an image, as it can take a lot of space - or at least mention the required space). People not using Docker on a regular basis would benefit from it (even if this information is, of course, available with a simple search - it would lower the entry level of using Docker).

Live Notebooks on binder

This is really an excellent possibility to get started directly with the software without a local installation and most examples worked (apart from the Tutorials). Please also include information on the main readme page (apart from the launch binder link at the top - for people who are not familiar with binder and this possibility...).

Documentation

The online documentation on https://uwgeodynamics.readthedocs.io/en/latest/ is really helpful! Is it possible to also include a link to this documentation directly in the paper?

The tutorials and examples provide a very good starting point to run the software. The provided example notebooks contain simulations of typical applications one would want to model with the software. The description in these notebooks is short, but sufficient to understand the context of the examples (typically also with references to related work).

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

I did not find community guidelines and possibilities for contribution. However, questions on the Issue tracker are addressed very quickly, so support is obvious (but again, some more information on the readme page would help).

Version

Version: Does the release version given match the GitHub release (v1.0.0)?

The DOI version on Zenodo is UWGeodynamics v2.7.4

Summary

Most of my comments refer to simply more information in the readme file. Great project (or projects - with underworld2) overall, looking forward to using it more in the future in my class!

@lheagy

This comment has been minimized.

Copy link
Member

lheagy commented Mar 29, 2019

Many thanks for your review @flohorovicic!!

@rbeucher, please take a look at @flohorovicic's comments above and let us know when you have addressed them. To help keep track of things, I would recommend that you open up an issue per item and link them here. Please let me know if you have any questions.

@rbeucher

This comment has been minimized.

Copy link

rbeucher commented Apr 1, 2019

Hi @flohorovicic

Thank you very much for taking the time to review our project. Your comments and suggestions were very useful.

I have open a series of issues on the repository where I quickly list the changes made.
They essentially affect the documentation in the README and on READTHEDOCS.
All the changes were squashed into one single commit. underworldcode/UWGeodynamics@3196752

The first point was about clarifying the difference between Underworld and UWGeodynamics.
I have added a short paragraph in the readme: underworldcode/UWGeodynamics#105

Installation

I have copied the Installation section from ReadTheDocs to the README file
I have also added some instructions on how to deal with docker images and containers
underworldcode/UWGeodynamics#106

Documentation
I have added a link to the documentation in the JOSS paper.
underworldcode/UWGeodynamics#108

Binder
I have added a short paragraph and a link to the binder
underworldcode/UWGeodynamics#107

Community guidelines

I have added some guidelines on how to contribute, raise issues and seek support in the README and
the readthedocs website.
underworldcode/UWGeodynamics#109

Version
The version is currently 2.7.5. We changed the version number system to match the version of the latest Underworld API supported. I have added a paragraph in the README and ReadTheDocs.
I have not made significant changes since v1.0 beside fixing bugs etc.
underworldcode/UWGeodynamics#110
@lheagy I am happy to draft a new released once the review process is finished.

Thanks again for your review and support!

Romain Beucher

@whedon

This comment has been minimized.

Copy link
Collaborator Author

whedon commented Apr 10, 2019


OK DOIs

- 10.1007/s00024-002-8738-3 is OK
- 10.1016/j.pepi.2007.06.009 is OK
- 10.1016/S0021-9991(02)00031-1 is OK
- 10.1371/journal.pone.0195557 is OK

MISSING DOIs

- None

INVALID DOIs

- None
@lheagy

This comment has been minimized.

Copy link
Member

lheagy commented Apr 10, 2019

@rbeucher: can you please generate a new release and archive your software on zenodo or similar? Please make sure that the title and author list on the zenodo archive match the JOSS paper. Once you have done this, please post the version number and doi here and we can proceed with accepting your paper.

@rbeucher

This comment has been minimized.

Copy link

rbeucher commented Apr 11, 2019

Hi @lheagy

Here it is

Romain Beucher, Louis Moresi, Julian Giordani, John Mansour, Dan Sandiford, Rebecca Farrington, … Sara Morón. (2019, April 11). UWGeodynamics: A teaching and research tool for numerical geodynamic modelling (Version v2.7.7). Zenodo. http://doi.org/10.5281/zenodo.2636105

@lheagy

This comment has been minimized.

Copy link
Member

lheagy commented Apr 11, 2019

@whedon set 2.7.7 as version

@whedon

This comment has been minimized.

Copy link
Collaborator Author

whedon commented Apr 11, 2019

OK. 2.7.7 is the version.

@lheagy

This comment has been minimized.

Copy link
Member

lheagy commented Apr 11, 2019

@whedon set 10.5281/zenodo.2636105 as archive

@whedon

This comment has been minimized.

Copy link
Collaborator Author

whedon commented Apr 11, 2019

OK. 10.5281/zenodo.2636105 is the archive.

@lheagy lheagy added the accepted label Apr 11, 2019

@lheagy

This comment has been minimized.

Copy link
Member

lheagy commented Apr 11, 2019

👋@openjournals/joss-eics, this submission is ready to be accepted! 🎉

@labarba

This comment has been minimized.

Copy link
Member

labarba commented Apr 11, 2019

@rbeucher : could you merge this in, with capitalization nitpicks?
underworldcode/UWGeodynamics#114

@rbeucher

This comment has been minimized.

Copy link

rbeucher commented Apr 11, 2019

I have merged it

@labarba

This comment has been minimized.

Copy link
Member

labarba commented Apr 11, 2019

@whedon accept

@whedon

This comment has been minimized.

Copy link
Collaborator Author

whedon commented Apr 11, 2019

Attempting dry run of processing paper acceptance...
@whedon

This comment has been minimized.

Copy link
Collaborator Author

whedon commented Apr 11, 2019

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

Can't find any papers to compile :-(

@whedon

This comment has been minimized.

Copy link
Collaborator Author

whedon commented Apr 11, 2019


OK DOIs

- 10.1007/s00024-002-8738-3 is OK
- 10.1016/j.pepi.2007.06.009 is OK
- 10.1016/S0021-9991(02)00031-1 is OK
- 10.1371/journal.pone.0195557 is OK

MISSING DOIs

- None

INVALID DOIs

- None
@labarba

This comment has been minimized.

Copy link
Member

labarba commented Apr 11, 2019

@whedon generate pdf

@whedon

This comment has been minimized.

Copy link
Collaborator Author

whedon commented Apr 11, 2019

Attempting PDF compilation. Reticulating splines etc...
@whedon

This comment has been minimized.

Copy link
Collaborator Author

whedon commented Apr 11, 2019

@labarba

This comment has been minimized.

Copy link
Member

labarba commented Apr 11, 2019

@whedon accept

@whedon

This comment has been minimized.

Copy link
Collaborator Author

whedon commented Apr 11, 2019

Attempting dry run of processing paper acceptance...
@whedon

This comment has been minimized.

Copy link
Collaborator Author

whedon commented Apr 11, 2019

Check final proof 👉 openjournals/joss-papers#617

If the paper PDF and Crossref deposit XML look good in openjournals/joss-papers#617, then you can now move forward with accepting the submission by compiling again with the flag deposit=true e.g.

@whedon accept deposit=true
@whedon

This comment has been minimized.

Copy link
Collaborator Author

whedon commented Apr 11, 2019


OK DOIs

- 10.1007/s00024-002-8738-3 is OK
- 10.1016/j.pepi.2007.06.009 is OK
- 10.1016/S0021-9991(02)00031-1 is OK
- 10.1371/journal.pone.0195557 is OK

MISSING DOIs

- None

INVALID DOIs

- None
@labarba

This comment has been minimized.

Copy link
Member

labarba commented Apr 11, 2019

@whedon accept deposit=true

@whedon

This comment has been minimized.

Copy link
Collaborator Author

whedon commented Apr 11, 2019

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

This comment has been minimized.

Copy link
Collaborator Author

whedon commented Apr 11, 2019

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

Here's what you must now do:

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

Any issues? notify your editorial technical team...

@labarba

This comment has been minimized.

Copy link
Member

labarba commented Apr 11, 2019

Congratulations @rbeucher, your JOSS paper is published!

Many thanks to our editor: @lheagy, and the reviewer: @flohorovicic — thanks also to @tth030 for the efforts! 🙏

@labarba labarba closed this Apr 11, 2019

@whedon

This comment has been minimized.

Copy link
Collaborator Author

whedon commented Apr 11, 2019

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

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

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

This is how it will look in your documentation:

DOI

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:

@rbeucher

This comment has been minimized.

Copy link

rbeucher commented Apr 12, 2019

Thanks @lheagy, @flohorovicic, @tth030, @labarba !!
Looking forward to the next submission! ;-)

@flohorovicic

This comment has been minimized.

Copy link
Collaborator

flohorovicic commented Apr 12, 2019

Congrats, @rbeucher! And it also was quite an interesting experience as a reviewer - thanks for such a great journal idea and concept, @labarba , @lheagy!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.