8000 [REVIEW]: Underworld3: Mathematically Self-Describing Modelling in Python for Desktop, HPC and Cloud · Issue #7831 · openjournals/joss-reviews · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[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

Open
editorialbot opened this issue Feb 20, 2025 · 45 comments
Assignees
Labels

Comments

@editorialbot
Copy link
Collaborator
editorialbot commented Feb 20, 2025

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

Status badge code:

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

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:

@editorialbot generate my 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

8000
@editorialbot
Copy link
Collaborator Author

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
Copy link
Collaborator Author
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

✅ OK DOIs

- 10.2172/2337606 is OK
- 10.21105/joss.01136 is OK
- 10.1016/j.advwatres.2011.04.013 is OK
- 10.1109/MCSE.2021.3059263 is OK
- 10.21105/joss.01797 is OK
- 10.7717/peerj-cs.103 is OK
- 10.1016/j.pepi.2007.06.009 is OK
- 10.1016/S0021-9991(02)00031-1 is OK

🟡 SKIP DOIs

- No DOI given, and none found for title: Achieving High Performance with Unified Residual E...

❌ MISSING DOIs

- 10.1109/mcse.2010.118 may be a valid DOI for title: Cython: The Best of Both Worlds

❌ INVALID DOIs

- None

@editorialbot
Copy link
Collaborator Author

Software report:

github.com/AlDanial/cloc v 1.98  T=0.49 s (343.0 files/s, 89514.0 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Python                          68           5610           4809          15044
Cython                           9           1122            973           2319
CSS                              4             39             19           2236
C/C++ Header                     5            289            690           1372
HTML                            22             55              6           1023
JavaScript                       7            103            100            826
Jupyter Notebook                 9              0           3767            723
Markdown                        12            218              0            548
YAML                            14             73             46            413
C                                2             40             69            359
TeX                              2             21             13            331
XML                              1              0              0            225
Text                             3             39              0            133
JSON                             3              0              0             79
Bourne Shell                     5             17             20             37
INI                              1              5              0             14
SCSS                             1              5              9              7
-------------------------------------------------------------------------------
SUM:                           168           7636          10521          25689
-------------------------------------------------------------------------------

Commit count by author:

   598	Louis Moresi
    82	julian
    71	Romain Beucher
    60	Ben Knight
    59	Julian Giordani
    39	jmansour
    31	Matthew G. Knepley
    17	john mansour
     7	Thyagarajulu Gollapalli
     5	jcgraciosa
     4	NengLu
     4	Tyagi
     3	Juan Carlos Graciosa
     3	Owen Kaluza
     1	Ben Mather
     1	Matthew Knepley
     1	Rebecca Farrington
     1	YangHaibin1102

@editorial
8000
bot
Copy link
Collaborator Author

Paper file info:

📄 Wordcount for paper.md is 1096

✅ The paper includes a Statement of need section

@diehlpk
Copy link
Member
diehlpk commented Feb 20, 2025

Hi @gassmoeller, @chennachaos I started the review here.

@editorialbot
Copy link
Collaborator Author

License info:

🟡 License found: Other (Check here for OSI approval)

@diehlpk
Copy link
Member
diehlpk commented Feb 20, 2025
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

✅ OK DOIs

- 10.2172/2337606 is OK
- 10.21105/joss.01136 is OK
- 10.1016/j.advwatres.2011.04.013 is OK
- 10.1109/MCSE.2021.3059263 is OK
- 10.21105/joss.01797 is OK
- 10.7717/peerj-cs.103 is OK
- 10.1016/j.pepi.2007.06.009 is OK
- 10.1016/S0021-9991(02)00031-1 is OK

🟡 SKIP DOIs

- No DOI given, and none found for title: Achieving High Performance with Unified Residual E...

❌ MISSING DOIs

- 10.1109/mcse.2010.118 may be a valid DOI for title: Cython: The Best of Both Worlds

❌ INVALID DOIs

- None

@lmoresi please have a look at the missing DOI.

@editorialbot
Copy link
Collaborator Author

👉📄 Download article proof 📄 View article proof on GitHub 📄 👈

@diehlpk
Copy link
Member
diehlpk commented Mar 7, 2025

@lmoresi please have a look at the missing DOI.

@diehlpk
Copy link
Member
diehlpk commented Mar 7, 2025

Hi @gassmoeller, @chennachaos how is your review going?

@gassmoeller
Copy link

Thanks for the reminder and sorry for being slow. This is high up on my list for next week.

@gassmoeller
Copy link
gassmoeller commented Mar 7, 2025

Review checklist for @gassmoeller

Conflict of interest

  • I confirm that I have read the JOSS conflict of interest (COI) policy and that: I have no COIs with reviewing this work or that any perceived COIs have been waived by JOSS for the purpose of this review.

Code of Conduct

General checks

  • Repository: Is the source code for this software available at the https://github.com/underworldcode/underworld3?
  • License: Does the repository contain a plain-text LICENSE or COPYING file with the contents of an OSI approved software license?
  • Contribution and authorship: Has the submitting author (@lmoresi) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
  • Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines
  • Data sharing: If the paper contains original data, data are accessible to the reviewers. If the paper contains no original data, please check this item.
  • Reproducibility: If the paper contains original results, results are entirely reproducible by reviewers. If the paper contains no original results, please check this item.
  • Human and animal research: If the paper contains original data research on humans subjects or animals, does it comply with JOSS's human participants research policy and/or animal research policy? If the paper contains no such data, please check this item.

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 functionality 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

  • Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
  • A statement of need: Does the paper have a section titled 'Statement of need' that clearly states what problems the software is designed to solve, who the target audience is, and its relation to other work?
  • State of the field: Do the authors describe how this software compares to other commonly-used packages?
  • Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
  • References: Is the list of references complete, and is everything cited appropriately that should be cited (e.g., papers, datasets, software)? Do references in the text use the proper citation syntax?

@gassmoeller
Copy link

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.

@diehlpk
Copy link
Member
diehlpk commented Mar 19, 2025

Hi @lmoresi please have a look at the above issues.

@lmoresi
Copy link
lmoresi commented Mar 20, 2025

Thanks (esp @gassmoeller !) - I missed the earlier notifications. On to it now

@diehlpk
Copy link
Member
diehlpk commented Mar 28, 2025

Hi @chennachaos how is your review going?

1 similar comment
@diehlpk
Copy link
Member
diehlpk commented Apr 8, 2025

Hi @chennachaos how is your review going?

@diehlpk
Copy link
Member
diehlpk commented Apr 8, 2025

Hi @lmoresi how is the progress on the open issues?

@lmoresi
Copy link
lmoresi commented Apr 17, 2025

More-or-less done. I just need to link the specific commits to the issues raised.

@lmoresi
Copy link
lmoresi commented Apr 22, 2025

@gassmoeller — here are the responses to your review comments:

Issues:

  1. [JOSS review] Licence.md refers to wrong licence file name underworldcode/underworld3#302 - fixed
  2. [JOSS Review] Local pytest run: 1 test fails underworldcode/underworld3#303 fixed in commit: underworldcode/underworld3@af61e0e
  3. [JOSS review] API documentation offline? underworldcode/underworld3#304 fixed (several commits) - links to API docs were broken after we reorganised the main / dev branches but did not update the README files in main/dev or joss-submission branches correctly.
  4. [JOSS review] Information about contributing underworldcode/underworld3#305: fixed in commit underworldcode/underworld3@73cbaea
  5. [JOSS review] Paper comments underworldcode/underworld3#306: updated paper.md to reflect comments from @gassmoeller. Specifically: Expanding the mathematics section to make it flow and to ensure all symbols are referenced / described correctly; Adding the missing "State of the Field" section

Checklist Items that are not issues:

  1. Automatic testing: the pytest suite is run for all PRs and also for the API documentation builds on main / dev prior to deployment. I have added a badge in the README which reflects the status of these tests.

This part of the response is complete. I am happy to have @gassmoeller close issues or request further changes.

L

@gassmoeller
Copy link

@editorialbot generate pdf

@editorialbot
Copy link
Collaborator Author

👉📄 Download article proof 📄 View article proof on GitHub 📄 👈

@gassmoeller
Copy link

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

@diehlpk
Copy link
Member
diehlpk commented Apr 24, 2025

Hi @chennachaos how is your review going?

@diehlpk
Copy link
Member
diehlpk commented Apr 24, 2025

@editorialbot check references

@editorialbot
Copy link
Collaborator Author
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

✅ OK DOIs

- 10.2172/2337606 is OK
- 10.1002/9780470050118.ecse159 is OK
- 10.1109/mcse.2010.118 is OK
- 10.21105/joss.01136 is OK
- 10.5281/ZENODO.10718860 is OK
- 10.1029/2007GC001719 is OK
- 10.1016/j.advwatres.2011.04.013 is OK
- 10.5194/gmd-15-5127-2022 is OK
- 10.1029/2011GC003551 is OK
- 10.1109/MCSE.2021.3059263 is OK
- 10.1093/gji/ggx195 is OK
- 10.1007/978-3-642-23099-8 is OK
- 10.21105/joss.01797 is OK
- 10.7717/peerj-cs.103 is OK
- 10.1016/B978-0-323-85733-8.00010-X is OK
- 10.1016/j.pepi.2007.06.009 is OK
- 10.1016/S0021-9991(02)00031-1 is OK
- 10.1002/2016GC006702 is OK
- 10.1016/B978-0-444-53802-4.00130-5 is OK

🟡 SKIP DOIs

- No DOI given, and none found for title: Achieving High Performance with Unified Residual E...

❌ MISSING DOIs

- 10.1016/0045-7825(87)90013-2 may be a valid DOI for title: The Finite Element Method: Linear Static and Dynam...
- 10.1016/b978-1-85617-633-0.00019-8 may be a valid DOI for title: The Finite Element Method: Its Basis and Fundament...

❌ INVALID DOIs

- None

@diehlpk
Copy link
Member
diehlpk commented Apr 24, 2025

Hi @lmoresi, Please have a look at the missing DOIs.

@diehlpk
Copy link
Member
diehlpk commented Apr 24, 2025

@lmoresi Why do you have to LICENSE.md and LICENSE.md? One has content and one is empty?

@lmoresi
Copy link
lmoresi commented May 1, 2025

Why do you have to LICENSE.md and LICENSE.md? One has content and one is empty?

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

❌ MISSING DOIs

  • 10.1016/0045-7825(87)90013-2 may be a valid DOI for title: The Finite Element Method: Linear Static and Dynam...
  • 10.1016/b978-1

doi/s missin for books (zenodo issue) - Fixed in commit e5b5dca9aeeaed0912328554de3f37e60e103d23

@lmoresi
Copy link
lmoresi commented May 2, 2025

@editorialbot check references

@editorialbot
Copy link
Collaborator Author
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

✅ OK DOIs

- 10.2172/2337606 is OK
- 10.1002/9780470050118.ecse159 is OK
- 10.1109/mcse.2010.118 is OK
- 10.21105/joss.01136 is OK
- 10.5281/ZENODO.10718860 is OK
- 10.1029/2007GC001719 is OK
- 10.1016/j.advwatres.2011.04.013 is OK
- 10.5194/gmd-15-5127-2022 is OK
- 10.1029/2011GC003551 is OK
- 10.1109/MCSE.2021.3059263 is OK
- 10.1093/gji/ggx195 is OK
- 10.1016/0045-7825(87)90013-2 is OK
- 10.1007/978-3-642-23099-8 is OK
- 10.21105/joss.01797 is OK
- 10.7717/peerj-cs.103 is OK
- 10.1016/B978-0-323-85733-8.00010-X is OK
- 10.1016/j.pepi.2007.06.009 is OK
- 10.1016/S0021-9991(02)00031-1 is OK
- 10.1002/2016GC006702 is OK
- 10.1016/B978-0-444-53802-4.00130-5 is OK

🟡 SKIP DOIs

- No DOI given, and none found for title: Achieving High Performance with Unified Residual E...

❌ MISSING DOIs

- None

❌ INVALID DOIs

- 10.1016/b978-1-85617-633-0.00019-7 is INVALID

@diehlpk
Copy link
Member
diehlpk commented May 2, 2025

Hi @chennachaos how is your review going?

@chennachaos
Copy link
chennachaos commented May 5, 2025

Review checklist for @chennachaos

Conflict of interest

  • I confirm that I have read the JOSS conflict of interest (COI) policy and that: I have no COIs with reviewing this work or that any perceived COIs have been waived by JOSS for the purpose of this review.

Code of Conduct

General checks

  • Repository: Is the source code for this software available at the https://github.com/underworldcode/underworld3?
  • License: Does the repository contain a plain-text LICENSE or COPYING file with the contents of an OSI approved software license?
  • Contribution and authorship: Has the submitting author (@lmoresi) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
  • Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines
  • Data sharing: If the paper contains original data, data are accessible to the reviewers. If the paper contains no original data, please check this item.
  • Reproducibility: If the paper contains original results, results are entirely reproducible by reviewers. If the paper contains no original results, please check this item.
  • Human and animal research: If the paper contains original data research on humans subjects or animals, does it comply with JOSS's human participants research policy and/or animal research policy? If the paper contains no such data, please check this item.

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 functionality 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

  • Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
  • A statement of need: Does the paper have a section titled 'Statement of need' that clearly states what problems the software is designed to solve, who the target audience is, and its relation to other work?
  • State of the field: Do the authors describe how this software compares to other commonly-used packages?
  • Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
  • References: Is the list of references complete, and is everything cited appropriately that should be cited (e.g., papers, datasets, software)? Do references in the text use the proper citation syntax?

@chennachaos
Copy link

Hi @diehlpk, I was waiting for the author to address the comments of the other reviewers so I can avoid repetitive questions/comments.
I have resumed my review. Will post my comments in a couple of days.

@chennachaos
Copy link

@editorialbot generate pdf

@editorialbot
Copy link
Collaborator Author

👉📄 Download article proof 📄 View article proof on GitHub 📄 👈

@chennachaos
Copy link
chennachaos commented May 5, 2025

@lmoresi my comments on the software paper as below. I am checking the other sections and will post comments soon.

  • References missing on lines 14, 21, 59. Check other missing references and fix them.

  • The statement of need section does not really answer the need part. It only states what underworld3 does and how it does. Add a couple of sentences on the need, answering questions like, Why underworld3?, What's unique about underworld3 that other similar software lack? What are its technical advantages?

  • The mathematical framework section needs careful reconsideration and rewriten correctly by considering the following.

    • How can F(u) be both strong form in (1) and weak form in (2)?
    • Eq(2) seems incorrect. Why is B with f term but not with F term? There should be N (basis functions matrix) for f, the source term. We don't typically see the constitutive matrix in the residual equation (1).
    • Write equations (2) and (3) together before explaining the symbols.
    • Why do you write the Jacobian in (3) when you actually compute it using SymPy in the software?

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.

@lmoresi
Copy link
lmoresi commented May 11, 2025

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

@knepley
Copy link
knepley commented May 11, 2025

I will respond to the four bullets in order:

  1. Yes, I agree that we should use a different symbol for the strong form in (1), maybe $\mathbb{F}$. This is not really a division between strong and weak, but rather a decomposition into parts that depend on the physics (f and F) and things that do not (everything else in $\mathcal{F}$. An identical decomposition can be found here, and to my knowledge this decomposition was first developed here.
  2. $B$ is synthesis operation, where a function is projected onto the finite element basis. That matrix is just the evaluation of each basis function for the cell on the quadrature points. The notation $N$ is a crazy thing from mechanical which we are not using. Similarly, D is the evaluation of the basis function gradients at the quadrature points.
  3. One is the residual operation and one is the Jacobian. I do not see the necessity of writing them together.
  4. Writing (3) allow us to see a mathematical form for the Jacobian, which can be reasoned about and manipulated. SymPy is a particular software strategy for implementing (3). These are categorically different things.
  5. FEncCSx and Firedrake use a different decomposition of the weak form. This is why libCEED could not be a backend for either of them.

@lmoresi
Copy link
lmoresi commented May 12, 2025

Updates in b0b9e88fdfd94885d9af157661d0d73192271d15:

  • Address broken references (regression !)
  • Add subscripts to functionals to separate weak and strong forms as pointed out by @chennachaos
  • Whatever the shortcomings of the notation in equations (2) and (3), they reference the original paper by @knepley et al and (we think) readers are best served looking at that document for details of these choices.
  • (2) and (3) can be adjacent if that is easier to follow. The paper has been modified accordingly.
  • We write out the Jacobian term since this is also part of the PETSc template and sympy computes the requested terms. In the code there is an almost exact correspondence between the form we write mathematically in (3) and the python code.

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

@chennachaos
Copy link

@lmoresi, more domain-specific need in the statement of need section.

@chennachaos
Copy link

@lmoresi, The documentation looks good and easy to follow. I suggest the following to improve it further.

  1. Add the governing equations, key details of formulations and the relevant time schemes used for the notebooks 5, 6, 7 and 8. Cite relevant papers and textbooks as references for the formulations used in the examples.
  2. Parallelisation is briefly discussed in the Parallel Execution section. Add parallel scalability in terms of strong and weak scaling for two examples: a scalar field problem (Poisson equation) and a vector field problem (Navier-Stokes). Also, some instructions on changing the PETSc solver options would be helpful.
  3. The section on Mesh Adaptation needs expanding a bit. The current image does not clearly depict the adapted mesh. Showing the original mesh and the adapted mesh would be helpful. I understand that it is low-level at present.
  4. https://www.underworlcode.org/ is broken.

@lmoresi
Copy link
lmoresi commented May 19, 2025

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

@diehlpk
3138 Copy link
Member
diehlpk commented Jun 2, 2025

@TY-00 and @lmoresi any update on the comments above?

@lmoresi
Copy link
lmoresi commented Jun 3, 2025

Yes, working on it, but the documentation revisions are quite lengthy to complete.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants
0