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

[JOSS] Review of the paper #64

Closed
thelfer opened this issue Dec 3, 2024 · 2 comments
Closed

[JOSS] Review of the paper #64

thelfer opened this issue Dec 3, 2024 · 2 comments

Comments

@thelfer
Copy link

thelfer commented Dec 3, 2024

Overall the paper is well written and easy to read. Except from a small remark below, the paper is fine for me.

However, the JOSS check list insists on two points Data sharing and Reproducibility. Are the tests presented in the paper available in the ngsPETSc sources or in the Firedrake sources ?

A small remark

The project has two seperate goals:

  1. All PETSc solvers benefits from NETGEN meshes and geometries.
  2. NGSolve users access to the wide array of linear, nonlinear solvers,
    and time-steppers available in PETSc.

The first statement shall be more precise as only solvers based on DMPLEX seem able to benefit from ngsPETSc (example: Firedrake). There are many solvers that use PETSc but don't use DMPLEX as their mesh backends (example: FEniCS). In fact, the paper only focus on Firedrake which somehow further weakens the genericity of this first
statement.

I think that the first statement shall be more humble without making it less interesting. I may propose something like:

ngsPETSc seamlessly integrates with PETSc DMPLEX, a feature that may benefit to any solver based in this solution. This paper illustrates this in the case of the Firedrake solver.

@UZerbinati
Copy link
Collaborator

Dear Thomas,

Thank you for taking the time to review our manuscript.

All examples in the paper are either in the ngsPETSc documentation docs/src or in paper/examples. In particular all code for reproducing the results in the manuscript is in the ngsPETSc repository.

We agree that we should modify our first statement and we have implemented your suggestion in PR #66 .

@thelfer
Copy link
Author

thelfer commented Dec 3, 2024

Perfect

@thelfer thelfer closed this as completed Dec 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants