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

[REVIEW]: solids4foam-v2.1: A toolbox for performing solid mechanics and fluid-solid interaction simulations in OpenFOAM #7407

Open
editorialbot opened this issue Oct 24, 2024 · 19 comments
Assignees
Labels

Comments

@editorialbot
Copy link
Collaborator

editorialbot commented Oct 24, 2024

Submitting author: @philipcardiff (Philip Cardiff)
Repository: https://github.com/solids4foam/solids4foam
Branch with paper.md (empty if default branch): joss
Version: v2.1
Editor: @kyleniemeyer
Reviewers: @MakisH, @mohd-afeef-badri
Archive: Pending

Status

status

Status badge code:

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

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

@MakisH & @mohd-afeef-badri, 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 @kyleniemeyer 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 @MakisH

📝 Checklist for @mohd-afeef-badri

@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

Software report:

github.com/AlDanial/cloc v 1.90  T=0.71 s (1271.2 files/s, 310890.4 lines/s)
--------------------------------------------------------------------------------
Language                      files          blank        comment           code
--------------------------------------------------------------------------------
C++                             364          25062          22528         103345
C/C++ Header                    348          15669          18459          23230
Markdown                         38           1816              0           4135
Bourne Again Shell              120            702            646           1931
m4                                7            136              0            772
Bourne Shell                     10            128            261            625
TeX                               1             15              0            177
YAML                              4             20              8            131
XML                               2             14              0            125
Fortran 77                        1              7              5             52
Dockerfile                        4             27             43             39
HLSL                              1              0              0              8
--------------------------------------------------------------------------------
SUM:                            900          43596          41950         134570
--------------------------------------------------------------------------------

Commit count by author:

   959	Philip Cardiff
    63	Danial Khazaei
    16	Iago Lessa de Oliveira
     6	philipcardiff
     3	Jeff Heylmun
     3	wyldckat
     2	Saber Mohammadi
     2	Zeljko Tukovic
     2	iagolessa
     2	scottlevie97
     1	Cyrille Bonamy
     1	Karen FitzGerald
     1	Philipp Milovic
     1	Sindeev Sergey
     1	emadtandis

@editorialbot
Copy link
Collaborator Author

Paper file info:

📄 Wordcount for paper.md is 668

✅ The paper includes a Statement of need section

@editorialbot
Copy link
Collaborator Author

Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

✅ OK DOIs

- 10.48550/arXiv.1808.10736 is OK
- 10.1002/nag.2361 is OK
- 10.1016/j.jmbbm.2022.105498 is OK
- 10.1016/j.compbiomed.2023.107178 is OK
- 10.1016/j.apm.2021.09.009 is OK
- 10.1002/nme.7302 is OK
- 10.1016/j.commatsci.2012.03.011 is OK
- 10.1016/j.cma.2013.09.008 is OK
- 10.1016/j.compstruc.2016.07.004 is OK
- 10.1002/nme.5345 is OK
- 10.21278/TOF.42301 is OK
- 10.1007/978-3-319-60846-4_1 is OK
- 10.1016/j.compstruc.2008.11.013 is OK

🟡 SKIP DOIs

- No DOI given, and none found for title: OpenFOAM.com
- No DOI given, and none found for title: OpenFOAM.org
- No DOI given, and none found for title: foam-extend project

❌ MISSING DOIs

- None

❌ INVALID DOIs

- None

@editorialbot
Copy link
Collaborator Author

License info:

🟡 License found: GNU General Public License v3.0 (Check here for OSI approval)

@kyleniemeyer
Copy link

👋 @philipcardiff @MakisH @mohd-afeef-badri the actual review will take place in here.

@editorialbot
Copy link
Collaborator Author

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

@philipcardiff
Copy link

FYI @iBatistic @ztukovic

@kyleniemeyer
Copy link

Hi @MakisH and @mohd-afeef-badri, just checking in on your reviews, since neither of you have yet initiated your checklists. Will you be able to work on these soon, since it's been a few months? Thanks!

@MakisH
Copy link

MakisH commented Dec 11, 2024

Hi @kyleniemeyer,
yes, I am planning to deliver the review this or next week.
Thank you for your patience, a lot has been going on over here lately.

@MakisH
Copy link

MakisH commented Dec 11, 2024

Review checklist for @MakisH

Conflict of interest

Code of Conduct

General checks

  • 🟢 Repository: Is the source code for this software available at the https://github.com/solids4foam/solids4foam?
  • 🟢 License: Does the repository contain a plain-text LICENSE or COPYING file with the contents of an OSI approved software license?
    • Yes, LGPL-3.0
  • 🟢 Contribution and authorship: Has the submitting author (@philipcardiff) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
    • Yes, he is the primary contributor.
  • 🟢 Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines
    • Yes, and the software already has users.
  • 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.
    • No data is described in the software paper.
  • Reproducibility: If the paper contains original results, results are entirely reproducible by reviewers. If the paper contains no original results, please check this item.
    • No data is described in the software paper.
  • 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.
    • No data is described in the software paper.

Functionality

  • 🟢 Installation: Does installation proceed as outlined in the documentation?
    • Yes. Building took me around 10min on my Ubuntu 22.04 laptop, using OpenFOAM v2312.
    • There are some optional file fixes suggested, which I skipped because of the system-wide installation of OpenFOAM.
  • Functionality: Have the functional claims of the software been confirmed?
    • I have successfully ran all featured tutorials and opened some issues suggesting improvements, as linked in the last section. The software clearly does what it claims overall, but I have not yet confirmed the individual methods.
  • Performance: If there are any performance claims of the software, have they been confirmed? (If there are no claims, please check off this item.)
    • There are some numerical and runtime performance claims in the fourth tutorial (3dTube). These would take substantial effort to reproduce, and I am currently missing some information (see last section). However, from my (little) experience on the field, I see nothing surprising: the plots seem as expected.

Documentation

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?
    • ⚠️ Suggestion: In point 1, I would add within OpenFOAM, similarly to point 2. ⏳ On the one hand, this is an advantage of solids4foam, on the other hand it separates it from preCICE & co. that target coupling to external codes.
    • ⚠️ Suggestion: I would see the point 4 of "guiding principles" (code quality) as controversial. On the one hand, it is difficult to argue about code quality, especially without concrete measures. On the other hand, following a style guide is only one tiny aspect of what comprises code quality. I would rather drop the point altogether.
  • State of the field: Do the authors describe how this software compares to other commonly-used packages?
    • ❌ This is missing
  • Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
    • ⚠️ Suggestion: The paper is generally easy to read, even though mostly structured around bullet-point lists. I would expect a bit more running text at parts (e.g., the opening of every section).
  • 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?
    • ⚠️ Suggestion: In some places, there are references to the methods (e.g., in line 39, the reference to the Degroote 2009 paper), and in other places the references are to later implementations/modifications (e.g., in the surrounding lines, the Tukovic 2018 paper). I am not sure what would be best here, but it feels inconsistent. At the same time, in the same section, I get the impression there is a mix of general mathematical methods with application-specific algorithms.
    • ⚠️ Suggestion: In other places (e.g., the thermo-fluid-solid interaction coupling), there are no references. Maybe for all of these uncited features, some URLs (just clickable, not necessarily printable) to the documentation, or some guidance to the configuration would help me (as a reviewer) confirm the claims, and a user to find their way.
    • ⚠️ Suggestion: The citation to the OpenFOAM Foundation is not rendered correctly. Maybe the editors can help here. Also, is Foam-extend the official writing of the project's name?
  • Further points:
    • ⚠️ Suggestion: There is a one-line "Tutorials" subsection, but this does not mention any examples. This would be helpful for a reader completely new to the tool to understand what this tool typically applies to.

Screenshots

🟢 First featured tutorial (hotSphere):

Screenshot from 2024-12-13 13-43-24

🟢 Second featured tutorial (cylinderInChannel):

Screenshot from 2024-12-13 13-57-54

🟢 Third featured tutorial (beamInCrossFlow): I could not run this tutorial with OpenFOAM v2312:

This case currently does not run with this OpenFOAM verion

Related issue: solids4foam/solids4foam#149

Update: The beamInCrossFlow now runs, but with OpenFOAM v2312 (without the file patches) I am getting a bit different results, looking at the value ranges. Is this because of the version difference, or because I am doing something wrong in ParaView?

Screenshot from 2024-12-23 08-59-03

Update 2: My results now look much closer to the updated image on the website. Still not exact, but I could imagine that there is something in the setup I am not reproducing (but I think I am only missing the optional fixes in OpenFOAM)

⏳ Fourth featured tutorial (3dTube):

Screenshot from 2024-12-13 17-47-54

I have not yet dived deep enough to understand what the default settings are and how to prepare comparable plots, but this is to document what results I have so far. Related issues:

@MakisH
Copy link

MakisH commented Dec 13, 2024

@kyleniemeyer I am through with my first iteration, and I have opened some issues and PRs to discuss individual aspects. I am happy to try more aspects, as needed.

@philipcardiff @iBatistic Let me know once you are ready for a second iteration.

@philipcardiff
Copy link

Thanks, @MakisH, for your comprehensive review and valuable suggestions!

@mohd-afeef-badri
Copy link

mohd-afeef-badri commented Dec 22, 2024

Review checklist for @mohd-afeef-badri

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/solids4foam/solids4foam?
  • 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 (@philipcardiff) 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?

@philipcardiff
Copy link

Update: The beamInCrossFlow now runs, but with OpenFOAM v2312 (without the file patches) I am getting a bit different results ⚠️, looking at the value ranges. Is this because of the version difference, or because I am doing something wrong in ParaView?

@MakisH Regarding this point, I have updated the image at https://www.solids4foam.com/tutorials/tutorial3.html, so it should now be consistent with your results. The reason for the difference is that there are two forms of the case (as described at the start of https://www.solids4foam.com/tutorials/tutorial3.html), and the previous image was for the first form of the case, whereas the case in solids4foam is actually set up to match the second form of the case (we made this change at some point, but I forgot to update this image). Along with updating the image, I have made a few minor edits at https://www.solids4foam.com/tutorials/tutorial3.html to clarify this point. Thanks!

@MakisH
Copy link

MakisH commented Jan 7, 2025

Thanks for the several updates in these tutorials, @philipcardiff. Note that the ranges in the image are still a bit different than the ranges in my results. Are these because of the optional fixes in backwardDdtScheme.C that I have not applied for v2312, or are the results with a different OpenFOAM version?

Looking at the state, I now only want to have another, deeper look at the 3dTube tutorial, and of course at the paper itself once the rest converges.

Anything else / more specific aspects you would like feedback on?

@kyleniemeyer
Copy link

Hello @mohd-afeef-badri, just checking in on the status of your review. Are you able to finish soon?

@mohd-afeef-badri
Copy link

Dear @kyleniemeyer, apologies for the slight delay in my review, the start of the year has been quite busy. I have successfully installed the toolbox and run the basic tests, and everything appears to be functioning well so far. I will need some additional time this week to thoroughly review the provided tests in detail.

Thank you for your patience.

@kyleniemeyer
Copy link

@mohd-afeef-badri that sounds great, thank you

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

5 participants