search

openjournals / joss-reviews

Reviews for the Journal of Open Source Software
Creative Commons Zero v1.0 Universal
725 stars 38 forks source link

[REVIEW]: Akantu: an HPC finite-element library for contact and dynamic fracture simulations #5253

Closed editorialbot closed 9 months ago

editorialbot commented 1 year ago

Submitting author: !--author-handle-->@nrichart<!--end-author-handle-- (Nicolas Richart) Repository: https://gitlab.com/akantu/akantu Branch with paper.md (empty if default branch): master Version: 5.0.5 Editor: !--editor-->@diehlpk<!--end-editor-- Reviewers: @vijaysm, @chennachaos Archive: 10.5281/zenodo.10668768

Status

status

Status badge code:

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

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

@vijaysm & @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 @chennachaos

📝 Checklist for @vijaysm

editorialbot commented 1 year ago

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 commented 1 year ago 
Software report:

github.com/AlDanial/cloc v 1.88  T=1.26 s (1224.7 files/s, 218502.9 lines/s)
--------------------------------------------------------------------------------
Language                      files          blank        comment           code
--------------------------------------------------------------------------------
C++                             454          15781          21871          58015
C/C++ Header                    545          14892          31207          49320
SVG                              43             40             48          38349
CMake                           191           1963           6426           8358
TeX                              41           1049            383           8027
Python                           68           1592            934           5047
GLSL                            111            565            301           2753
reStructuredText                 18           1380           1888           2566
YAML                             15             69             53            762
Markdown                          4            106              0            382
C                                 3            103             62            340
Bourne Shell                     32             69              8            267
Dockerfile                        7             68             25            221
XSLT                              1              7              6            120
Lisp                              1             11              2             87
PHP                               3             18              9             82
Bourne Again Shell                1             25             12             44
make                              3             11             33             31
JSON                              4              0              0             29
TOML                              1              0              0             11
--------------------------------------------------------------------------------
SUM:                           1546          37749          63268         174811
--------------------------------------------------------------------------------

gitinspector failed to run statistical information for the repository
editorialbot commented 1 year ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1002/nme.5339 is OK
- 10.1016/j.actamat.2015.01.003 is OK
- 10.1016/j.finel.2015.02.003 is OK
- 10.1002/nme.4837 is OK
- 10.1016/j.compstruc.2020.106459 is OK
- 10.1617/s11527-018-1236-6 is OK
- 10.1016/j.ijsolstr.2017.02.014 is OK
- 10.1016/j.cemconres.2022.106744 is OK
- 10.1016/j.engstruct.2022.115427 is OK
- 10.1016/j.cemconres.2022.106744 is OK
- 10.1016/j.cemconres.2022.106974 is OK
- 10.1016/j.ijsolstr.2020.10.010 is OK
- 10.1016/j.engstruct.2019.04.026 is OK
- 10.1016/j.cemconres.2017.03.014 is OK
- 10.1016/j.engfracmech.2017.09.015 is OK
- 10.1016/j.cemconres.2016.08.014 is OK
- 10.1002/nme.6816 is OK
- 10.1016/j.cemconres.2018.03.020 is OK
- 10.1016/j.conbuildmat.2021.122244 is OK
- 10.1103/PhysRevE.95.043002 is OK
- 10.1016/j.mechmat.2014.03.009 is OK
- 10.1073/pnas.1517545113 is OK
- 10.1007/s11249-012-9920-0 is OK
- 10.1002/nme.4898 is OK
- 10.1016/j.jmps.2015.12.014 is OK
- 10.1016/j.ijsolstr.2015.01.008 is OK
- 10.1007/s10704-017-0239-6 is OK
- 10.1016/j.jmps.2013.10.007 is OK
- 10.1016/j.ijsolstr.2020.06.040 is OK
- 10.1016/j.cma.2019.04.006 is OK
- 10.1002/nme.6705 is OK
- 10.1016/j.jmps.2021.104607 is OK

MISSING DOIs

- None

INVALID DOIs

- None
editorialbot commented 1 year ago

Wordcount for paper.md is 995

editorialbot commented 1 year ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

chennachaos commented 1 year ago

Review checklist for @chennachaos

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

nrichart commented 1 year ago

@editorialbot generate pdf

editorialbot commented 1 year ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

vijaysm commented 1 year ago

Review checklist for @vijaysm

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

diehlpk commented 1 year ago

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

vijaysm commented 1 year ago

I started an issue page with some comments related to installation instructions at the repository gitlab page. I should be able to get the rest of the installation verified on a linux machine in the next couple of days.

diehlpk commented 1 year ago

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

vijaysm commented 1 year ago

Sorry @diehlpk was on vacation but back this week. Should be able to complete the full review by end of next week.

diehlpk commented 1 year ago

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

chennachaos commented 1 year ago

Hi @diehlpk. Been quite occupied with other stuff. I will try to finish the review by the end of this month.

chennachaos commented 1 year ago

@editorialbot generate pdf

editorialbot commented 1 year ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

vijaysm commented 1 year ago

I've been able to successfully build and install on Ubuntu. Not on OSX yet, but less of a concern at the moment since I have it working somewhere. The rest of the review should proceed relatively quick. @diehlpk

vijaysm commented 1 year ago

@nrichart I do not see anything explicit in the documentation or README about contributor guidelines. Can you please let me know if I missed something? 

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
nrichart commented 1 year ago

@vijaysm this part was missing, as you've seen it was added in the same merge request as the instructions to compile on MacOS X that should get merged, as soon as the CI is finished

chennachaos commented 1 year ago

Hi @nrichart and @diehlpk,

I have thoroughly checked the submission (code repository, documentation and the paper). Too many capabilities are claimed that are not seen in the documentation. I think this library is still in work in progress and is not yet ready for consideration for publication. I am providing some pointers below to help the authors improve the software.

Documentation

The library must be improved considerably before it can be considered a substantial scholarly effort. The library might be capable, but the documentation does not reflect the capabilities, both in terms of physics, different FE aspects and HPC details.

I do not think the heat transfer module belong in this library as that capability is not claimed in the paper. It is too short anyway; missing some information and also missing several key boundary conditions, e.g. flux, convection and radiation. Moreover, only one time integration scheme (forward Euler) is considered. Without at least flux and convection BCs, and with only one time integration scheme, the heat transfer module can be discounted from the library. If the authors want to keep it, then they should consider improving the capabilities and documentation for the heat transfer modules.

The same issues with structural mechanics modules. The capability shown in the documentation is elementary. It must be improved substantially. Show some examples of the 3D beam and shell elements.

No examples of plastic and visco-plastic constitutive laws. No examples of large deformations. No examples of contact constraints (including rate and state friction). No examples of one-dimensional elements embedded in a three-dimensional mesh (e.g. reinforcements in concrete) No examples of interaction between contact and cohesive elements (residual crack shear strength).

Not clear about the phase field module in this library. The documentation is too short and examples/tutorials are missing.

Summarising, there are no examples and step-by-step tutorials demonstrating each capability in the documentation. I understand some tutorials are available here but there is no description. The video file is missing for the tutorial on phase field models. Moreover, all the tutorials are in 2D while the paper claims 3D HPC capability. There should be separate sections dedicated to examples/tutorials showcasing the capabilities and steps in detail. Some tutorials must show 3D examples. The authors may refer to MoFEM for their tutorials for reference.

The documentation hardly discusses the parallel capabilities, procedures to run in parallel using test cases, and scalability studies. How is the domain decomposition handled? How are the results post-processed? What parallel solvers and preconditioners are available and how to change these options? The document must address all these questions since this library is claimed as an HPC library.

Software paper

diehlpk commented 1 year ago

Hi @nrichart,

please have a look ath the comments above and provide detailed feedback.

Also some timeline to address these, since that will require quite some effort.

nrichart commented 1 year ago

@editorialbot generate pdf

editorialbot commented 1 year ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

nrichart commented 1 year ago

@diehlpk @chennachaos we will reply asap, to the different points that where raised and give you a time estimate for the changes that we will do. I already addressed the easiest point, the missing figure in the paper.

nrichart commented 1 year ago

The library must be improved considerably before it can be considered a substantial scholarly effort. The library might be capable, but the documentation does not reflect the capabilities, both in terms of physics, different FE aspects and HPC details.

We thank the reviewer for the comment. We address the reviewer's concerns to the best of our ability below.

I do not think the heat transfer module belong in this library as that capability is not claimed in the paper. It is too short anyway; missing some information and also missing several key boundary conditions, e.g. flux, convection and radiation.

We believe that there is no point in removing a working model from the code, even if there is no mention of it in the software paper. JOSS guidelines do not imply that we must enumerate all the capabilities of the code. After all, Akantu is a SolidMechanics oriented code with a Lagrangian formulation, where advection equations would not appear. For SolidMechanics applications involving temperature (plastic dissipation and others), it is generally sufficient to have a diffusion model. Still, we understand the possible confusion, we have an open merge request for renaming the HeatTransferModel into a PoissonModel (MR!50), which should remove any ambiguity (with fluid mechanics naming).

Moreover, only one time integration scheme (forward Euler) is considered. Without at least flux and convection BCs, and with only one time integration scheme, the heat transfer module can be discounted from the library.

As mentionned, we do not wish to remove a useful model (plasticity coupled with temperature field for instance). Nevertheless we noticed that the documentation is not up to date (thank you for pointing it out): Indeed we have several first order time integration schemes within Akantu's features, that are not in the documentation.

If the authors want to keep it, then they should consider improving the capabilities and documentation for the heat transfer modules.

We shall improve the documentation and clarify capabilities in the naming of classes involved.

The same issues with structural mechanics modules. The capability shown in the documentation is elementary. It must be improved substantially. Show some examples of the 3D beam and shell elements.

The features provided for structural elements are elementary, yet they are useful and used in both research projects and teaching. We will expand on the module's documentation and aim for a more exhaustive description of capabilities. If the state remains problematic we can remove the feature claim from the software paper as it is not what makes Akantu interesting and useful in an academic context.

No examples of plastic and visco-plastic constitutive laws. No examples of large deformations. No examples of contact constraints (including rate and state friction). No examples of one-dimensional elements embedded in a three-dimensional mesh (e.g. reinforcements in concrete) No examples of interaction between contact and cohesive elements (residual crack shear strength).

There are examples of mentionned physics in the source code repository in the examples/ directory. We will add a description of all of them in the documentation, therefore providing the information of which features are demonstrated in each example.

Not clear about the phase field module in this library. The documentation is too short and examples/tutorials are missing.

The phase field model is a work in progress, in Akantu, but also at the research level. The current implementation is a single formulation, but active developments may provide a fine tuning of the known models in the literature. The phase-field tutorial is still under development, but the examples (in the source repository) are stable. For all these reasons, we did not mention Phase-Field fracture in the capabilities of the code in the software paper. We will mention more clearly in the documentation which feature is stable or under development.

Summarising, there are no examples and step-by-step tutorials demonstrating each capability in the documentation. I understand some tutorials are available here but there is no description. The video file is missing for the tutorial on phase field models. Moreover, all the tutorials are in 2D while the paper claims 3D HPC capability. There should be separate sections dedicated to examples/tutorials showcasing the capabilities and steps in detail. Some tutorials must show 3D examples. The authors may refer to MoFEM for their tutorials for reference.

The documentation mentions the tutorials and the tutorials are step-by-step presentation of the capabilities of the code. In particular, the possibilities offered with Cohesive-Element for fracture, that we think make the strength of Akantu, are well demonstrated in the attached tutorial. In order to make a better link with the documentation, we will provide a succinct description of the tutorials in the documentation (a short one as otherwise it would be redundant with the tutorial content), and the examples provided in the source repository. The tutorials are limited to 2D as it was important that these can run on the Renku (web-oriented resource on demand) platform. This allows to experiment Akantu without the burden to proceed to the installation of dependencies. Again, the examples provided in the source, have 3D simulation setups, so that adding a list of examples in the documentation should emphasize better the 3D examples, as well as the parallel computations possibilities.

The documentation hardly discusses the parallel capabilities, procedures to run in parallel using test cases, and scalability studies. How is the domain decomposition handled? How are the results post-processed? What parallel solvers and preconditioners are available and how to change these options? The document must address all these questions since this library is claimed as an HPC library.

We thank the reviewer for raising this point. We will add a section in the documentation about the parallel capabilities and procedure to run simulations.

Software paper

* There are quite a few FEM libraries with HPC capabilities. So, the _statement of need_ should also discuss the unique physics capabilities of the library.

We agree that there are a few open source FEM libraries with HPC capabilities. That is why we tried to emphasize the capability of Akantu to insert extrinsic cohesive elements in a distributed, parallel setting. In order to take into account the reviewer's point, we will rewrite the text to make clearer the uniqueness of Akantu in this research area.

* No image for Figure 1.

This is corrected, thanks for pointing this out.

* Difficult to understand Figure 2 without some figures for the problems under consideration.

We will add a figure showing the mechanical problem setup, which was considered for the scalability test.

To summarize, we will clarify the documentation to present the different examples/tutorials and separate the parts under development from the ones that are more stable. We estimate that it will take ~1 month based on the current schedules of the team involved in these changes. The work will be done in the merge request MR!97, so that the reviewers can follow the changes.

Once finished, we will merge the branch and inform the reviewers.

diehlpk commented 1 year ago

@nrichart I paused the review until you are done with the proposed changes.

Please notify me here to resume with the review.

diehlpk commented 1 year ago

@vijaysm, @chennachaos due to the major changes requested, I will pause the review. I will notify you when we proceed.

diehlpk commented 1 year ago

@nrichart can you please provide me with your update?

diehlpk commented 1 year ago

@nrichart can you please provide me with your update?

nrichart commented 1 year ago

@diehlpk sorry we did not manage to advance as much as we expected. The modifications will not be ready before the end of August.

diehlpk commented 1 year ago

@editorialbot remind @nrichart in three weeks

editorialbot commented 1 year ago

Reminder set for @nrichart in three weeks

diehlpk commented 1 year ago

@nrichart Could you please report your progress?

diehlpk commented 1 year ago

@nrichart Could you please report your progress?

nrichart commented 1 year ago

@diehlpk sorry for the delay in my answer. I got other priorities added to my schedule and could not work as much as I wanted on the review. The only think I manage to do is the renaming of the HeatTransferNodel to DiffusionModel MR!48 I still have to do the documentation changes. I am trying to "recruit" some of the lab PhD students to give me a hand. We might change the author list if this is ok with you ? and the version of the code also.

We will not be able to finish before end of September, sorry once again for the extra delays (and my miscalculations in planning)

diehlpk commented 1 year ago

I am trying to "recruit" some of the lab PhD students to give me a hand. We might change the author list if this is ok with you ? and the code version.

Sure, you can add a new author, assuming this person will make some contribution.

Yes, you can update the version. Anyways, you need to release a new version after the JOSS review.

diehlpk commented 1 year ago

@editorialbot remind @nrichart in three weeks

editorialbot commented 1 year ago

Reminder set for @nrichart in three weeks

editorialbot commented 1 year ago

:wave: @nrichart, please update us on how things are progressing here (this is an automated reminder).

diehlpk commented 1 year ago

@nrichart Can you please report on your progress?

editorialbot commented 1 year ago

:wave: @nrichart, please update us on how things are progressing here (this is an automated reminder).

nrichart commented 1 year ago

The code side of the changes is done. We are working on the documentation of the examples and tutorials. We are still aiming to finish the changes for end of the month (next week).

diehlpk commented 1 year ago

@nrichart Are you done with the changes?

Can I open the review and ask the reviewers to proceed?

nrichart commented 1 year ago

@diehlpk I am waiting on the CI to pass on the MR!97 to merge it, and the feedbacks of the second author on the paper.

nrichart commented 1 year ago

@editorialbot generate pdf

editorialbot commented 1 year ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

nrichart commented 1 year ago

@editorialbot generate pdf

editorialbot commented 1 year ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

diehlpk commented 1 year ago

Hi @vijaysm, @chennachaos I removed the pause label, since the authors worked on your issues.

Could you please start with your review again?