-
-
Notifications
You must be signed in to change notification settings - Fork 41
[REVIEW]: Underworld3: Mathematically Self-Describing Modelling in Python for Desktop, HPC and Cloud #7831
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
Comments
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:
For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:
|
|
Software report:
Commit count by author:
|
Paper file info: 📄 Wordcount for ✅ The paper includes a |
Hi @gassmoeller, @chennachaos I started the review here. |
License info: 🟡 License found: |
@lmoresi please have a look at the missing DOI. |
@lmoresi please have a look at the missing DOI. |
Hi @gassmoeller, @chennachaos how is your review going? |
Thanks for the reminder and sorry for being slow. This is high up on my list for next week. |
Review checklist for @gassmoellerConflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
|
I have finished my first pass of the review and opened the issues linked to above. In summary I think this will be a very valuable contribution to JOSS and the software is worth publishing if the issues I raised above are addressed. |
Hi @lmoresi please have a look at the above issues. |
Thanks (esp @gassmoeller !) - I missed the earlier notifications. On to it now |
Hi @chennachaos how is your review going? |
1 similar comment
Hi @chennachaos how is your review going? |
Hi @lmoresi how is the progress on the open issues? |
More-or-less done. I just need to link the specific commits to the issues raised. |
@gassmoeller — here are the responses to your review comments: Issues:
Checklist Items that are not issues:
This part of the response is complete. I am happy to have @gassmoeller close issues or request further changes. L |
@editorialbot generate pdf |
@lmoresi Thanks for addressing my comments. All my substantial comments have been addressed and I have updated my checklist above accordingly. I have one remaining minor suggestion for punctuation changes in underworldcode/underworld3#306 but from a functional standpoint this is ready from my side. |
Hi @chennachaos how is your review going? |
@editorialbot check references |
|
Hi @lmoresi, Please have a look at the missing DOIs. |
@lmoresi Why do you have to |
@diehlpk LICENSE.md is a symbolic link to the LICENCE.md file. We consistently use the British / Australian spelling of license (verb) / licence (noun) in our publications and documentation but GitHub thinks the code is unlicenced if the LICENSE file is missing - or at least it did when we first started uw3.
doi/s missin for books (zenodo issue) - Fixed in commit e5b5dca9aeeaed0912328554de3f37e60e103d23 |
@editorialbot check references |
|
Hi @chennachaos how is your review going? |
Review checklist for @chennachaosConflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
|
Hi @diehlpk, I was waiting for the author to address the comments of the other reviewers so I can avoid repetitive questions/comments. |
@editorialbot generate pdf |
@lmoresi my comments on the software paper as below. I am checking the other sections and will post comments soon.
Also, briefly explains how underworld3 is different from FEniCSx and Firedrake, which also follow a similar philosophy of code generation through Python and JIT compilation. |
@knepley - can you provide a comment on the mathematical framework issues raised by @chennachaos - specifically about the potential problems with the terminology in your paper on the PETSc point wise functions / weak form. The current revision of our paper in pdf form is in the repo here: https://github.com/underworldcode/underworld3/blob/joss-submission/docs/joss-paper/paper.pdf and the equation under suspicion is (2). @knepley - I think the fact that F(u) is in both the strong and weak form is the exact point of the formulation outlined in your paper, can you comment though ? |
I will respond to the four bullets in order:
|
Updates in b0b9e88fdfd94885d9af157661d0d73192271d15:
@chennachaos asks for clarification of the Fenics / Firedrake codes. We think this is appropriately handled in the statement of the field section (missing in the original document) which compares both Fenics / Firedrake as well as other codes with scripted front ends that automatically generate weak forms. Regarding the statement of need section, @chennachaos, are you requesting more domain-specific need (ie. what is missing in geodynamics that underworld3 fixes) or are the changes requested by @gassmoeller sufficient ? |
@lmoresi, more domain-specific need in the statement of need section. |
@lmoresi, The documentation looks good and easy to follow. I suggest the following to improve it further.
|
@TY-00 — can you look at the comments above as you go through the Quickstart guide and then we'll chat about how to make this easier for a day-one beginner to look at. |
Yes, working on it, but the documentation revisions are quite lengthy to complete. |
Uh oh!
There was an error while loading. Please reload this page.
Submitting author: @lmoresi (Louis Moresi)
Repository: https://github.com/underworldcode/underworld3
Branch with paper.md (empty if default branch): joss-submission
Version: 0.99.0b
Editor: @diehlpk
Reviewers: @gassmoeller, @chennachaos
Archive: Pending
Status
Status badge code:
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
@gassmoeller & @chennachaos, 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:
The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @diehlpk 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 @gassmoeller
📝 Checklist for @chennachaos
The text was updated successfully, but these errors were encountered: