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

Comments on your JOSS paper review #426

Closed
thelfer opened this issue Jul 15, 2020 · 11 comments
Closed

Comments on your JOSS paper review #426

thelfer opened this issue Jul 15, 2020 · 11 comments

Comments

@thelfer
Copy link

thelfer commented Jul 15, 2020

This documents is part of the review of the paper submitted to the Journal of Open Source Software. See openjournals/joss-reviews#2369.

The paper describes the scikit-fem python package which is dedicated to the assembly of the matrices and vectors resulting from the finite element discretization of partial differential equations. More precisely, the library allows the user to focus on the weak form of the differential equation and the choice of the finite element library (several families are provided).

The paper illustrates the library with three examples, grouped in a single figure:

  • a linear elastic contact problem.
  • a H(curl)-conforming model problem.
  • a resolution of the Navier–Stokes equations for different Reynolds numbers using Taylor–Hood element.

General remarks and questions

  • The paper is easy to read. I don't have particular remarks on the quality of english used.
  • The paper is very short. I understand that the task is to library is very narrow. However, many things are missing to fully understand the interest and the limits of the library. For example:
    • The library do provide a way to handle Dirichlet boundary conditions by elimination. This shall be at least mentioned. I imagine that the treatment of more complex conditions, such as multiple points constraints in mechanics, have to be treated by hand after the assembly, isn't it ?
    • Following the previous remark, it is not clear how the library could be used in this way. For example, is the mesh connectivity clearly described (how the unknowns are numbered) ?
    • The authors shall describe how the library could be used in non linear cases. In particular, the contact problem given in the first example is indeed non linear.
    • Can non uniform coefficients be handled ? To make my question clearer, would I be able to treat:
      • a linear elastic problem with a non uniform Young Modulus.
      • a heat transfer equation with a non uniform heat source Those non uniform coefficients are dealt with in UFL using the notion of functions spaces. Is there anything equivalent in scikit-fem ?
  • The authors claims that the assembly, while made in pure python, is "fast enough" ? This shall be made more explicit. Do the authors mean that the assembly takes a negligible time compared to the resolution of the resulting linear system ?
  • The authors claims that more than 30 examples illustrate the use of the library. I did not find any page that summarises those examples. It would be very useful for new users. I mean something similar to what the MFEM developers provide: https://mfem.org/examples/
  • Would it work in MPI-like processes ? Do you plan any extension of the package in that matter or is it out of its scope ?.

Specific comments

Quadrature points

UFL offers functional spaces at quadrature points. This functional spaces are relevant when dealing with non linear constitutive equations in solid mechanics. Is there anything similar in scikit-fem ?

Figures

With which package have the figures been generated ?

Second example

The description of the second example is very sparse.

Dependencies

Some examples depends on external packages which are not installed with scikit-fem. An appropriate list of dependencies would be appreciated.

review.pdf

@kinnala
Copy link
Owner

kinnala commented Jul 15, 2020

Thanks for valuable feedback. Some clarifying questions:

Would it work in MPI-like processes ? Do you plan any extension of the package in that matter or is it out of its scope ?.

It's currently out of scope. Do you ask this to be discussed in the paper?

The description of the second example is very sparse.

Do you mean the following example?
https://kinnala.github.io/scikit-fem-docs/example2.html

UFL offers functional spaces at quadrature points. This functional spaces are relevant when dealing with non linear constitutive equations in solid mechanics. Is there anything similar in scikit-fem ?

I'm not sure I recognize the concept but I have used scikit-fem library to solve, e.g., nonlinear (Neohookean) elasticity. Is this (functional spaces at quadrature points) something related to plasticity?

@thelfer
Copy link
Author

thelfer commented Jul 16, 2020

Would it work in MPI-like processes ? Do you plan any extension of the package in that matter or is it out of its scope ?.

It's currently out of scope. Do you ask this to be discussed in the paper?

It would be nice to have a better view of the limitations and perspectives of the package. But that's only a point of discussion among (many) others.

@thelfer
Copy link
Author

thelfer commented Jul 16, 2020

The description of the second example is very sparse.

Do you mean the following example?
https://kinnala.github.io/scikit-fem-docs/example2.html

No. I mean the second example of the paper: "a H(curl)-conforming model problem."

@thelfer
Copy link
Author

thelfer commented Jul 16, 2020

UFL offers functional spaces at quadrature points. This functional spaces are relevant when dealing with non linear constitutive equations in solid mechanics. Is there anything similar in scikit-fem ?

I'm not sure I recognize the concept but I have used scikit-fem library to solve, e.g., nonlinear (Neohookean) elasticity. Is this (functional spaces at quadrature points) something related to plasticity?

In a way, it is related to plasticity. More precisely, I am the main author of MFront (http://tfel.sourceforge.net/) which is a set of domain specific languages dedicated to advanced constitutive equations (such as plasticity). This explains my interest on this topic.

Advanced constitutive equations describes the evolution of internal state variables (elastic strain, dislocation densities, equivalent plastic strain, back-strains, damage, etc....). One the values of the internal state variables are known, the stresss can be deduced, so the knowledge of those values are required at the quadrature points.

@kinnala
Copy link
Owner

kinnala commented Jul 16, 2020

Advanced constitutive equations describes the evolution of internal state variables (elastic strain, dislocation densities, equivalent plastic strain, back-strains, damage, etc....). One the values of the internal state variables are known, the stresss can be deduced, so the knowledge of those values are required at the quadrature points.

I think I found a discussion of the concept in Chapter 26 of https://launchpadlibrarian.net/83776282/fenics-book-2011-10-27-final.pdf

I'll try to read it through and see if there is anything comparable in scikit-fem.

@kinnala
Copy link
Owner

kinnala commented Jul 20, 2020

The authors claims that more than 30 examples illustrate the use of the library. I did not find any page that summarises those examples. It would be very useful for new users. I mean something similar to what the MFEM developers provide: https://mfem.org/examples/

There is now a gallery of examples at https://scikit-fem.readthedocs.io/en/latest/listofexamples.html .

@kinnala
Copy link
Owner

kinnala commented Jul 28, 2020

Some examples depends on external packages which are not installed with scikit-fem. An appropriate list of dependencies would be appreciated.

There is now an explicit discussion of installation dependencies and test dependencies (all examples are tested) in the README. Do you find it sufficient?

@kinnala
Copy link
Owner

kinnala commented Jul 28, 2020

The authors claims that the assembly, while made in pure python, is "fast enough" ? This shall be made more explicit. Do the authors mean that the assembly takes a negligible time compared to the resolution of the resulting linear system ?

There is now also a simple performance benchmark in the beginning of the README. Do you find it sufficient?

@kinnala
Copy link
Owner

kinnala commented Jul 28, 2020

Following the previous remark, it is not clear how the library could be used in this way. For example, is the mesh connectivity clearly described (how the unknowns are numbered) ?

We added documentation on the numbering of the DOF's at https://scikit-fem.readthedocs.io/en/latest/bcs.html

@thelfer
Copy link
Author

thelfer commented Aug 5, 2020

There is now also a simple performance benchmark in the beginning of the README. Do you find it sufficient?

That's fine for me. Thanks

@thelfer
Copy link
Author

thelfer commented Aug 5, 2020

I do appreciate our answers to my remarks and the time you spend on it. Closing this issue which is no more meaningful and start finalizing the review.

@kinnala kinnala closed this as completed Aug 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants