search

openjournals / joss-reviews

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

[REVIEW]: FeenoX: a cloud-first free no-fee no-X uniX-like finite-element(ish) computational engineering tool #5846

Closed editorialbot closed 4 months ago

editorialbot commented 1 year ago

Submitting author: !--author-handle-->@gtheler<!--end-author-handle-- (German Theler) Repository: https://github.com/seamplex/feenox/ Branch with paper.md (empty if default branch): Version: v1.0 Editor: !--editor-->@Kevin-Mattheus-Moerman<!--end-editor-- Reviewers: @vijaysm, @AnjaliSandip, @chennachaos Archive: 10.5281/zenodo.10819606

Status

status

Status badge code:

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

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 & @AnjaliSandip & @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 @Kevin-Mattheus-Moerman 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 @vijaysm

📝 Checklist for @AnjaliSandip

📝 Checklist for @chennachaos

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=0.61 s (1122.7 files/s, 260462.5 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
SVG                             73            163              8          73661
C                              126           6923           8990          24148
Markdown                       106           5139              2          15711
GLSL                           188            924            349           7132
Bourne Shell                   129           1072            342           4092
C/C++ Header                    18           1046            947           4091
TeX                              5             52             69           1642
C++                              2             53             29            581
m4                               6             80              9            544
XML                              1             10              6            371
make                             4             21             13            297
Python                           4             79              6            238
HTML                             1             21              8            234
Lua                              8             27             55            185
YAML                            12             17              1            147
JSON                             1              0              0            137
Elm                              1             11              0             88
vim script                       1             10             13             30
awk                              3              3              1             23
-------------------------------------------------------------------------------
SUM:                           689          15651          10848         133352
-------------------------------------------------------------------------------

gitinspector failed to run statistical information for the repository
editorialbot commented 1 year ago

Wordcount for paper.md is 667

editorialbot commented 1 year ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.2172/1968587 is OK
- 10.1007/978-1-4612-1986-6_8 is OK
- 10.1145/1089014.1089019 is OK
- 10.1016/j.anucene.2018.01.021 is OK

MISSING DOIs

- None

INVALID DOIs

- None
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:

Kevin-Mattheus-Moerman commented 1 year ago

@vijaysm, @AnjaliSandip, @chennachaos thanks for agreeing to review. I understand that each of you do not have time at the moment, but can start in mid-October/November. That is okay. I'll send a reminder to you around mid-October. In the mean time, this review issue is open and ready for your to start whenever you want.

Whenever you a ready, you can comment @editorialbot generate my checklist here and out bot will generate your review checklist to get started.

Thanks for your help!

Kevin-Mattheus-Moerman commented 1 year ago

@gtheler as you can see, the reviewers I was able to find cannot start immediately. I hope that is okay. It can be tricky finding reviewers during the summer (lots of holiday conflicts) and again in September (start of semester for many academics). Thanks for your patience. I hope that the review process will be fairly smooth once we get started.

gtheler commented 1 year ago

Thanks @Kevin-Mattheus-Moerman . Indeed, I did not take the summer into account because when I first submitted the draft it was winter for me (I'm from Argentina). I understand how difficult it is to thoroughly review a submission like this so let's make sure we make a good job. I'm sure we'll work out the review procees all together. Thank you and all the volunteers that agreed to review.

Kevin-Mattheus-Moerman commented 12 months ago

@gtheler sorry the "summer" comment was a bit "Northern Hemisphere"-ist of me. Fingers crossed the review should pick up according to schedule and run smoothly.

Kevin-Mattheus-Moerman commented 11 months ago

@vijaysm, @AnjaliSandip, 👋 thanks again for agreeing to review this work. Since you mentioned you could start ~mid-October, I am just pining to see if that is still the plan and you can pick this up now. As mentioned above, you can call @editorialbot generate my checklist to generate your review check list here.

Thanks for your help! Let me know if you have any questions.

vijaysm commented 11 months ago

@Kevin-Mattheus-Moerman I'm on travel until end of next week. I'll start the review after that and expect to make quick progress then.

AnjaliSandip commented 10 months ago

Review checklist for @AnjaliSandip

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

vijaysm commented 10 months ago

Review checklist for @vijaysm

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

AnjaliSandip commented 10 months ago

This article presents an open-source cloud-native computational tool, FeenoX, that provides a mathematical framework to solve partial differential equations (PDEs). The software was launched over a year ago with a decent commit history. The software paper is well written. I suggest the following minor revisions.

  1. Include a file titled “LICENSE” in the repository (not in the sub-folders) with the license information
  2. Include the installation instructions in the README file
  3. In the README file, combine 1, 2 and 3. Include installation instructions/quickstart along with download instructions. Licensing information should be removed from the README file.
  4. Include clear guidelines for third parties wishing to contribute to the software.
  5. The summary in the software paper could be revised for a diverse, non-specialist audience.

Once the above have been addressed, I support publication.

AnjaliSandip commented 10 months ago

@Kevin-Mattheus-Moerman I have completed the review.

Kevin-Mattheus-Moerman commented 10 months ago

@gtheler could you please respond to these comments? ☝️

gtheler commented 10 months ago

This article presents an open-source cloud-native computational tool, FeenoX, that provides a mathematical framework to solve partial differential equations (PDEs). The software was launched over a year ago with a decent commit history. The software paper is well written. I suggest the following minor revisions.

Thank you very much for your time @AnjaliSandip . I am very excited about this review.

  1. Include a file titled “LICENSE” in the repository (not in the sub-folders) with the license information

The license file is called COPYING, following the GNU recommendations. I have added a symbolic link named LICENSE to the existing license file in commit e4ee9c5f24fb57e4477960ac7829c4b1a5b23586

  1. Include the installation instructions in the README file

The common installation instructions (i.e. cloning the git repo and compiling) are already in the README at https://github.com/seamplex/feenox/blob/main/README#L568 There is also a link to the more detailed installation instructions at https://github.com/seamplex/feenox/blob/main/doc/compilation.md Can you please be more specific about what you would like to see in the README?

  1. In the README file, combine 1, 2 and 3. Include installation instructions/quickstart along with download instructions. Licensing information should be removed from the README file.

What do you mean by "combine 1, 2 and 3"? Do you want me to remove the headers? The brief download section is already in the README: https://github.com/seamplex/feenox/tree/main#download with a link to more detailed instructions at https://seamplex.com/feenox/download.html

I really would like to keep the licensing information in the README. It is not a repeated paragraph from the license but my own summary of what free software means.

  1. Include clear guidelines for third parties wishing to contribute to the software.

There are some guidelines here https://seamplex.com/feenox/doc/programming.html compiled from the markdown sources at https://github.com/seamplex/feenox/blob/main/doc/programming.md Would you like more particular instructions?

5. The summary in the software paper could be revised for a diverse, non-specialist audience.

Fair enough. Any suggestion about which parts should be improved?

I am looking forward to getting this work published. Again, thank you very much.

Once the above have been addressed, I support publication.

AnjaliSandip commented 10 months ago

@gtheler

  1. Looks good.
  2. This is satisfactory.
  3. I suggest the README file be organized in the following manner - (a) About the software, (b) quickstart (or how to use it), © an example with results, (d)licensing, and (e)clear guidelines for third parties wishing to Contribute to the software, Report issues or problems with the software and Seek support. You have much of the above, except for how the information is organized.
  4. I suggest you explicitly include how third parties can contribute under “Further information”
  5. Terms such as ODEs/DAEs without the full form provided, or core-level steady-state neutronic, finite element? Etc. I will suggest you have a diverse, non-specialist audience read this and make changes based on their feedback.
gtheler commented 10 months ago

Dear @AnjaliSandip , I have updated the README as per your recommendations in this commit: https://github.com/seamplex/feenox/commit/3b9f4ad7614e69c93600bbd27fd1c35d4e4d90d8

About point #5, let me see how I can improve the summary in the paper. Thanks.

AnjaliSandip commented 10 months ago

@gtheler README looks good

gtheler commented 10 months ago

@editorialbot generate pdf

editorialbot commented 10 months ago

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

gtheler commented 10 months ago

Dear @AnjaliSandip I have updated the summary of the paper in this commit: https://github.com/seamplex/feenox/commit/07ffa9ef3e25f7381539a21d003a2dc7d1a42f8b

AnjaliSandip commented 10 months ago

@gtheler 3,4, and 5 resolved. I support the software paper for publication.

vijaysm commented 10 months ago

@gtheler I just finished reading the submitted paper and have couple of suggestions below.

  1. "access element shape functions and their derivatives": While I understand what you are trying to convey, it may be better to have a small snippet or example of how the local element assembly or point-wise Gauss assembly is invoked. This might be the kernel of the assembly routine and an example for say homogeneous laplacian here may suffice. This is important since FeenoX uses FE. Additionally, some description of what types of FE basis and orders are supported may be helpful as well.
  2. In terms of references, it might be useful to compare the original Fenics/Dolfin suite (https://fenicsproject.org/) that provides somewhat similar functionality to describe the problem as a weak form statement using [UFL](https://fenics.readthedocs.io/projects/ufl/en/latest/#:~:text=The%20Unified%20Form%20Language%20(UFL,notation%20close%20to%20mathematical%20notation.). A detailed comparison may not be needed - but contrasting the approach used in UFL vs FeenoX Software Design Specification may be interesting.
gtheler commented 10 months ago

Thanks for your suggestions @vijaysm . I have implemented them in https://github.com/seamplex/feenox/commit/c1fe176c2ad6ee06a02ab6edcbcb50bd955f59b9

Kevin-Mattheus-Moerman commented 10 months ago

@chennachaos do you think you can get started at this point? You can call @editorialbot generate my checklist to generate your checklist.

chennachaos commented 10 months ago

Hi @Kevin-Mattheus-Moerman. Yes, I am back in the UK. I am going to start the review shortly. Since other reviewers have completed it already, mine should not take longer.

chennachaos commented 10 months ago

Review checklist for @chennachaos

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

chennachaos commented 10 months ago

@editorialbot generate pdf

editorialbot commented 10 months ago

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

chennachaos commented 10 months ago

Hi @Kevin-Mattheus-Moerman, After carefully going through the paper, repository, documentation and examples available, I believe that the library is not substantial in its extent, capabilities and usefulness.

Because of the way it is organised and too much unnecessary text in many places, it was a quite difficult library to review. I am not entirely sure what the author is trying to convey with the comparisons with Markdown right at the beginning of the description. I acknowledge that the author might have their own philosophy about how FeenoX is different. The use of scripts as inputs is not new.

While the idea of offering a general mathematical framework to solve PDEs with a bunch of entry points (as C functions) where new types of PDEs (e.g. electromagnetism, fluid mechanics, etc.) can be added to the set of problems FeenoX can solve is good, development and testing of such solvers requires a substantial effort from the users/developers. Solving PDEs by writing weak forms in FEniCS and FreeFEM is totally different and much easier than developing the stiffness matrices from scratch as needed in this library. While the library contains solver capabilities only for a few basic problems (mostly steady-state/static, transient heat transfer but mostly linear problems) and offers FEM ingredients for further developments, the development of solvers for other PDEs, particularly for nonlinear problems, is not as simple as indicated in the paper. So, in the context of developing new solvers, the library is not as flexible as claimed. Scalability claims are also not justified.

I am not convinced about the statement of need and the usefulness of this library when compared with similar libraries like FEniCS and FreeFEM. I do not recommend the library in its current form for publication in JOSS. I think the library has potential but requires additional developments in solver capabilities and higher-level functionality that can lessen the burden of solver development. The documentation requires some clarity and cleaning; there is too much text. Moreover, there should be examples explaining the development of new solvers rather than input scripts of test cases for existing solvers.

gtheler commented 10 months ago

Please allow me to explain all the issues you kindly raised. I'm very sad to see that I was not clear enough so as not to be able to make the points I wanted to make with FeenoX. Most of the comments in the review miss both the software and the paper's spirit. I'll need some time to ellaborate.

gtheler commented 10 months ago

Dear @chennachaos

Here is my elaborated answer to your kind review. If you all allow me, I will comment your review and then summarize all the ideas at the end.

After carefully going through the paper, repository, documentation and examples available, I believe that the library is not substantial in its extent, capabilities and usefulness.

Thanks for spending weekend time going through the FeenoX website. I know you already made a choice about publication, I respect it and it is fine. I just want to get help to convey the ideas behind FennoX that I think make them unique, at least in the free and open-source world as there's no other tool that fulfills the SDS that I know of. Maybe the failure in communication comes from the fact that I am not a native English speaker (I am from Argentina) because when I talk (in Spanish) with other colleagues (from the nuclear academy and industry) they do understand me and get my points. So please, I do humbly ask all four of you @Kevin-Mattheus-Moerman @AnjaliSandip @vijaysm @chennachaos to tell me where the communication fails.

In particular, about the extent, capabilities and usefulness mentioned.

Because of the way it is organised and too much unnecessary text in many places, it was a quite difficult library to review.

I agree with the objection about too much text, but it should boil down to the SRS & SDS. However, here is a first point that triggers a cascade of further misunderstandings. FeenoX is not a library. The SDS explicitly asks that users need no compile anything to use the tool. FeenoX is an executable that reads one or more input files. It is designed primarily as a end-user tool, not as a developer tool. Its main focus is industry while at the same time providing a way for academics, researchers, developers (which might be a small fraction of the main users) to do interesting stuff. I will come back again to the users and developer distinction below.

I am not entirely sure what the author is trying to convey with the comparisons with Markdown right at the beginning of the description. I acknowledge that the author might have their own philosophy about how FeenoX is different. The use of scripts as inputs is not new.

Let me work backwards. Not only I do know that "the use of scripts as inputs is not new" but I encourage its usage as opposed to the "everything needs a GUI" idea that clashes with the Unix philosophy, which is the philosophy under which FeenoX was developed and it is the one that is "different" with respect to most of the other computational tools out there. Unix philosophy asks explicitly for ASCII formats which can be tracked with Git. They also ask for the "rule of silence" and the "rule of economy" which I'll briefly explain below. So now to the comparison between Word, TeX and markdown and traditional FEM solvers, FEM libraries and FeenoX. The analogy only works if somebody used both Word, Markdown and TeX, but I hope we are on the same s about the combination of easy of use and powerfulness between Word and TeX and the fact that Markdown lands in the middle and wins the battle.

In this comparison, traditional industrial point-and-click FEM software would be like Word. They are easy but not so flexible. In the "free" world I would mention CalculiX and CodeAster. Advanced libraries (i.e. routines that need to be compiled, used and linked to solve problems) would be like TeX. They are very flexible but extremely hard to use. I a thinking in Sparselizard or MoFEM. The idea is tha FeenoX would sit in the middle. As requested in the SDS, regular users would not need to compile anything. And a simple problem (like the NAFEMS LE10 case shown in the paper) would need a very simple input file. More complex cases (like the one shown in the examples page of the website) would need more complex input files.

Both CalculiX and CodeAster (and Elmer etc) read input files. But these input files are way too complicated (and make no sense to me) even for very simple cases. In https://seamplex.com/feenox/tests/nafems/le10/ I try to explain how I managed to create input files to solve the very simple NAFEMS-LE10 case with these two standard tools. It was an un-needed nightmare, and the resulting input files are not Git-friendly. The input files reported here directly scare me: https://cofea.readthedocs.io/en/latest/benchmarks/001-thick-plate/tested-codes.html Also, the LE10 test page is outdated but I can show that FeenoX is way faster than the other tools out there for this case.

While the idea of offering a general mathematical framework to solve PDEs with a bunch of entry points (as C functions) where new types of PDEs (e.g. electromagnetism, fluid mechanics, etc.) can be added to the set of problems FeenoX can solve is good, development and testing of such solvers requires a substantial effort from the users/developers.

I could not agree more. I cannot help feeling my communication skills are way too bad here. First of all, once again, users and developers are not the same group of people. FeenoX should be "easy" for "users" solving "simple" problems. More complex problems do not have to be necesarily easy (but easier than other tools). Second, adding support for a new PDE will need substantial effort from the developer. I don't think how it cannot need substantial effort. I just wanted to convey that this effort is smaller that creating a whole solver from scratch. Please, even if the paper is not published, help me out to communicate this concept in a more efficient way.

Solving PDEs by writing weak forms in FEniCS and FreeFEM is totally different and much easier than developing the stiffness matrices from scratch as needed in this library.

I don't agree. Again, differentiation between users and developers is central here. Solving PDEs with FEnniCS and FreeFEM is way more complex than with FeenoX. Once a PDe has been added to Feenox by developers, users don't need to write the matrices. They have to build a proper input file with PROBLEM thermal. I can tell you that a significant fraction of "users" do not know what a boundary condition is, let a alone a weak form.

While the library contains solver capabilities only for a few basic problems (mostly steady-state/static, transient heat transfer but mostly linear problems) and offers FEM ingredients for further developments, the development of solvers for other PDEs, particularly for nonlinear problems, is not as simple as indicated in the paper.

Again, I did not want to say "simple" but "simpler than..." Regarding the "few basic problems". The mechanical solver can be run in a quasi-static fashion, for instance reading a temperature distribution computed in a different mesh but same geometry solving a non-linear transient heat conduction problem. Use case: computation of ASME linearized stress history needed to evaluate environmentally-assisted fatigue due to thermal transients in piping of nuclear power plants at interfaces between carbon and stainless steel classes. One can also add seismic loads too: https://github.com/seamplex/piping-asme-fatigue

The tutorials also show a nice application of the ellipticity of the Laplace equation, both steady and transient: https://www.linkedin.com/feed/update/urn:li:activity:7117082910381232128/

To complete, just this last week I was thanked by a colleague that wrote his Master's thesis about thermal optimization of a reflector in an integrated reactor. He just had to focus on the python part of the topological optimization algorithm while FeenoX could take care of solving the non-linear heat conduction equation and write the heat fluxes needed to evalaute the cost function in a way that would have been way more complicated using another FEM solver.

So, in the context of developing new solvers, the library is not as flexible as claimed.

I respectfully disagree. First, it is not a library. Second, it can solve a wide plethora of problems, some of the illustrated in the examples sections. Third, have you tried to add neutron transport with the method of discrete ordinates to FEniCSx?

Scalability claims are also not justified.

It is true that parallelization has a lot of room for improvement. But the fact that FeenoX can solve neutron transport with large N over unstructured grids by splitting the problem into several MPI ranks in different hosts is already a sound scalability claim.

I am not convinced about the statement of need and the usefulness of this library when compared with similar libraries like FEniCS and FreeFEM.

Maybe here is the main misunderstanding: FeenoX is not comparable to FEniCS nor FreeFEM. Not even to Sparselizard. If something, to CalculiX and CodeAster.

I do not recommend the library in its current form for publication in JOSS.

Fair enough. If this paper is not published I will not re-submit it.

I think the library has potential but requires additional developments in solver capabilities and higher-level functionality that can lessen the burden of solver development.

I agree. Yet, the main point is not about "solver developers" but about "users".

The documentation requires some clarity and cleaning; there is too much text.

I agree, but it should boil down to the SRS and the SDS.

Moreover, there should be examples explaining the development of new solvers rather than input scripts of test cases for existing solvers.

Fair enough. The examples are for mostly for "users". First, let me put some numbers: I expect a "developer" to be 1 per 1,000 to 10,000 "users". So I would rather start with examples for the most common type of user, which is industrial users. Second, any explicit example about how to add a new PDE will be either simple or particular to that PDE. So an actual example would be exactly adding the PDE myself (which is what I am doing with an academic professor that needs to model electrical currents in biological tissue). The instructions are

  1. take a look at the laplace directory for a basic elliptic operator
  2. open a new thread in the Github discussions to see how to proceed

The rule of silence says that if Unix tools should not clutter up output with unnecessary output. The rule of economy says that engineering time is way more expensive than CPU time. If one combines these two rules, the only plausible solution for a FEM solver is to have 100%-user-defined output, which is what FeenoX has. Solvers dating back from 1970s have large output files where every single result is written to. This makes it far more time consuming for the engineer (user) to filter out the important results than to directly spawn new executions that generate only the desired results. This is something I have not seen in industrial solvers (yes of course in libraries).

If you want, we can prepare a test case like the LE10 problem, solve it with different tools and measure both the CPU and memory needed. I am confident that my FeenoX will be one of the top scorers.

Finally, note that currently I am an employee of said top-notch FEA company. Therefore, FeenoX is a weekend project and the goal of this publication was to give the project more visibility so at the end of the day more people (both users and developers) could join me. If this paper is not published in its present form with further small comments, I will not re-submit it again as I would be writing very much the same claims.

Once again, thank you very much.

gtheler commented 10 months ago

@Kevin-Mattheus-Moerman what are the next steps? Is this submission considered rejected?

vijaysm commented 10 months ago

I have not yet fully finished my review, but the instructions were easy enough to replicate a clean installation in a docker container. I was able to run most of the examples as well without issues. @gtheler

Does FeenoX provide a groundbreaking new methodology or solver? IMHO, probably not. However, I still find that it offers a flexible infrastructure with lua scripts as input, to compute linear solutions of Laplace-like problems to slightly more complicated discrete-ordinates treatment of Boltzmann transport in multiple dimensions. While libraries such as FreeFEM or Fenics do offer better and more efficient functionality to match said capabilities, FeenoX's modules may be useful for quick prototyping or computing sensitivity to parameter variations (once a problem is setup apriori). If documented better, it may also be a simple tool to use for students before writing their own computational solver implementations.

The main deficiency that I do see (and agree with @chennachaos here) is that FeenoX does not seem to showcase any solvers for nonlinear systems. Additionally, with PETSc as a dependency, I do not see any details about the underlying parallel solvers, precondition setups in the documentation, or even example usage of nonlinear schemes for any of the problems (with the --non-linear option for FeenoX executable). I had to directly pass in command-line arguments (like --log_view, --ksp_monitor etc) to understand the solver setup used in PETSc, and modify options further to change the preconditioners etc. This should be better documented as well, especially if the intended audience are users unfamiliar with PETSc options.

I am also confused by the terminology "cloud-first". I assumed this meant that it could be deployed on cloud computing platforms like AWS. However, this is unclear and misleading.

Overall, I believe that it may be beneficial to accept this submission if the author can address the deficiency in the documentation to clarify better the classes of problems supported, usage options and current limitations, and expected behavior when running examples (including ways to improve the solver). I also find that several of the example pages have dead links to images and other resources, which should be fixed.

gtheler commented 10 months ago

Thanks for your time @vijaysm .

I have not yet fully finished my review, but the instructions were easy enough to replicate a clean installation in a docker container. I was able to run most of the examples as well without issues. @gtheler

Good. Honestly I never did thay myself because I don't like docker, but that was exactly the spirit of the code.

Does FeenoX provide a groundbreaking new methodology or solver? IMHO, probably not.

Agree.

However, I still find that it offers a flexible infrastructure with lua scripts as input, to compute linear solutions of Laplace-like problems to slightly more complicated discrete-ordinates treatment of Boltzmann transport in multiple dimensions. While libraries such as FreeFEM or Fenics do offer better and more efficient functionality to match said capabilities, FeenoX's modules may be useful for quick prototyping or computing sensitivity to parameter variations (once a problem is setup apriori). If documented better, it may also be a simple tool to use for students before writing their own computational solver implementations.

That's exactly what the Word/Markdown/TeX comparison means with GUI solvers/FeenoX/specific FEM libraries. Thanks for understanding.

The main deficiency that I do see (and agree with @chennachaos here) is that FeenoX does not seem to showcase any solvers for nonlinear systems.

Agree. So far the thermal solver is the one that fully handles non-linear solves. Non-linearity might arise from

  1. temperature-dependent BCs
  2. temperature-dependent volumetric power sources

  3. temperature-dependent conductivities The tests directory covers all three cases, although it might be true that there are no examples of these problems:

Let's do as follows. What if I wrote a tutorial (like the two that are already available at https://seamplex.com/feenox/doc/tutorials/) for the thermal case, including all the non-linear features? Would that showcase enough non-linear cases?

Additionally, with PETSc as a dependency, I do not see any details about the underlying parallel solvers, precondition setups in the documentation, or even example usage of nonlinear schemes for any of the problems (with the --non-linear option for FeenoX executable).

https://www.seamplex.com/feenox/doc/feenox-manual.html#problem

The --non-linear option should not be used. FeenoX should detect if a problem is linear or not by itself. If you ran any of the cases I listed about, all of them should use SNES so --snes_view will show something. In linear cases, --snes_view will be empty. In those cases where one wants to overwrite what it detected, then use --linear or --non-linear.

I had to directly pass in command-line arguments (like --log_view, --ksp_monitor etc) to understand the solver setup used in PETSc, and modify options further to change the preconditioners etc. This should be better documented as well, especially if the intended audience are users unfamiliar with PETSc options.

One can control the basic options for PETSc with the PROBLEM keyword: https://www.seamplex.com/feenox/doc/feenox-manual.html#problem If one does not want to write PETSc options in the command line every time, one can use the PETSC_OPTIONS keyword: https://www.seamplex.com/feenox/doc/feenox-manual.html#petsc_options

The usage of PETSc command-line options is encouraged by both FeenoX and the PETSc team. I don't see what's wrong with that.

I am also confused by the terminology "cloud-first". I assumed this meant that it could be deployed on cloud computing platforms like AWS. However, this is unclear and misleading.

Fair enough. It would need a whole document for that. Let me sumarize:

a. FeenoX is the back-end at www.caeplex.com so it proves it can be deployed to AWS by a single-person company (i.e. myself) b. the SRS and the SDS do talk a little bit about the idea of "cloud first." It does not mean just that it can run over ssh. It involves CI, deployment, interaction with other tools, I/O, means of reporting status remotely, etc. I can further explain because I now work at a FEM company that acquired a startup company that developed a cloud-based FEM system from scratch (that startup hired me because of what I had done in CAEplex, so I know what I'm talking about).

Overall, I believe that it may be beneficial to accept this submission if the author can address the deficiency in the documentation to clarify better the classes of problems supported, usage options and current limitations, and expected behavior when running examples (including ways to improve the solver).

Agree. To do what you propose in a general way is a many man-months job, and my original idea was to publish FeenoX in JOSS as it is so as to give publicity and get more people onboard. The guidelines mentioned that the software did not have to be fully finished to be accepted for publication.

But if you can summarize these deficiencies in a few bullets I can add to the documentation in a few weeks, that would be a start.

I also find that several of the example pages have dead links to images and other resources, which should be fixed.

Can you please tell me where so I can correct them?

chennachaos commented 9 months ago

@gtheler, as @vijaysm has also pointed out there are several issues. I am happy to revisit once @vijaysm completes his review.

gtheler commented 9 months ago

Thanks @chennachaos . Looking forward to @vijaysm 's review.

Kevin-Mattheus-Moerman commented 9 months ago

@gtheler did you implement the changes you hinted at to @vijaysm e.g. the thermal tutorials featuring non-linear applications? Are we waiting for you to implement some requested changes?

gtheler commented 9 months ago

No, I did not @Kevin-Mattheus-Moerman . Not sure what the next steps and what the choices are. I really would like the paper to be published, so I am eager for suggestions.

If someone thinks writing a non-linear thermal tutorial will help me achieve my goal, then I will hapilly do it (not before the weekend, as FeenoX is a free-time project).

Kevin-Mattheus-Moerman commented 9 months ago

@vijaysm could you get back to @gtheler on required changes, e.g. those non-linear tutorials hinted at or any other aspects?

vijaysm commented 9 months ago

That's exactly what the Word/Markdown/TeX comparison means with GUI solvers/FeenoX/specific FEM libraries.

You should rephrase this in the paper. The comparison of a requirement specification to markdown or TeX is irrelevant. It creates more confusion rather than providing motivations for the FeenoX design.

Agree. So far the thermal solver is the one that fully handles non-linear solves. Non-linearity might arise from

Fair enough. However, I do not see any examples or mention in the documentation related to how one might go forward with setting up a nonlinear problem. I was able to run the cases you mentioned in the tests folder and verify that SNES solvers are being used.

The tests directory covers all three cases, although it might be true that there are no examples of these problems:

Create explicit examples that showcase the nonlinear problem solver that addresses all three conditions to describe the features.

Let's do as follows. What if I wrote a tutorial (like the two that are already available at https://seamplex.com/feenox/doc/tutorials/) for the thermal case, including all the non-linear features? Would that showcase enough non-linear cases?

Yes.

I also want to point out that under the examples link, several hyperlinks are dead. For example, https://www.seamplex.com/feenox/doc/laplace.html#how-to-solve-a-maze-without-ai, https://www.seamplex.com/feenox/doc/thermal.html#thermal-slabs, https://www.seamplex.com/feenox/doc/mechanical.html#parallelepiped-whose-youngs-modulus-is-a-function-of-the-temperature to name a few.

The --non-linear option should not be used. FeenoX should detect if a problem is linear or not by itself.

The documentation mentions that one can provide --linear or --non-linear argument to force FeenoX to use a nonlinear or just a linear solver. But in the examples that I tried, forcing a linear problem to be nonlinear did not work as expected. So it is not clear how these options are implemented if they do not exactly force a behavior.

One can control the basic options for PETSc with the PROBLEM keyword: https://www.seamplex.com/feenox/doc/feenox-manual.html#problem

This is useful. Please make this link more prominent as I did not notice this before. If you are also using a "prefix" for the solver options, then it will be helpful to mention this in the documentation as well.

The usage of PETSc command-line options is encouraged by both FeenoX and the PETSc team. I don't see what's wrong with that.

There is nothing wrong with that, and that is the advantage of PETSc-based codes. However, without clearly specifying how a user needs to provide these options, the flexibility that is available from command-line options will remain under or unutilized.

a. FeenoX is the back-end at www.caeplex.com so it proves it can be deployed to AWS by a single-person company (i.e. myself)

I did not have the time to experiment with this, but the capability is interesting. This can be highlighted more prominently in the repository README.md to emphasize how FeenoX can use a cloud-based deployment without installation or computational requirement hurdles for users.

b. the SRS and the SDS do talk a little bit about the idea of "cloud first." It does not mean that it can run over ssh. It involves CI, deployment, interaction with other tools, I/O, means of reporting status remotely, etc. I can further explain because I now work at a FEM company that acquired a startup company that developed a cloud-based FEM system from scratch (that startup hired me because of what I had done in CAEplex, so I know what I'm talking about).

This particular aspect has no direct implication of being "cloud"-friendly. Almost every good library or application that can be deployed on Docker supports good CI, deployment, status reporting, etc. I suggest focusing on CAEPlex to emphasize cloud computing.

I am on travel the rest of this week and may not be able to devote more time. But hopefully, this gives a sense of what I am looking for to complete the review. Please let me know if you have more questions.

vijaysm commented 9 months ago

From README.md

Since it is free (as in freedom) and open source, contributions to add features (and to fix bugs) are welcome. In particular, each kind of problem supported by FeenoX (thermal, mechanical, modal, etc.) has a subdirectory of source files which can be used as a template to add new problems, as implied in the “community-contributed problems” bullet above (rules of modularity and extensibility). See the documentation for details about how to contribute.

I suggest fixing the "documentation" link to point to a dedicated page about guidelines to contributing users. Do you support pull requests? How are reviews done for new changes before acceptance? Github workflows are not used for CI and so how do you decide if new changesets are valid? Some details regarding these aspects will be helpful if you intend to receive contributions from new users.

gtheler commented 9 months ago

Thank you all. I now understand what the source of most of the misunderstandings is.

I'll work on all of the suggested items and will update both the paper, the documentation and the website.

gtheler commented 9 months ago

Dear @vijaysm . I have not finished the thermal tutorial but the non-linear part should be there. Let me know if I'm on the right track.

Let's do as follows. What if I wrote a tutorial (like the two that are already available at https://seamplex.com/feenox/doc/tutorials/) for the thermal case, including all the non-linear features? Would that showcase enough non-linear cases?

Yes.

I haven't finished it, but here's what I've written during the weekend:

I still have to fill in the linear and transient parts. Does the non-linear section make sense for you guys?

I also want to point out that under the examples link, several hyperlinks are dead. For example, https://www.seamplex.com/feenox/doc/laplace.html#how-to-solve-a-maze-without-ai, https://www.seamplex.com/feenox/doc/thermal.html#thermal-slabs, https://www.seamplex.com/feenox/doc/mechanical.html#parallelepiped-whose-youngs-modulus-is-a-function-of-the-temperature to name a few.

Thanks. They were missing a directory in the URL. It should be now fixed. Anyway, the source of the examples is https://www.seamplex.com/feenox/examples/

The --non-linear option should not be used. FeenoX should detect if a problem is linear or not by itself.

The documentation mentions that one can provide --linear or --non-linear argument to force FeenoX to use a nonlinear or just a linear solver. But in the examples that I tried, forcing a linear problem to be nonlinear did not work as expected. So it is not clear how these options are implemented if they do not exactly force a behavior.

There was a small bug regarding --linear and --non-linear. I fixed it and added tests cases. They should now work as expected. The thermal tutorial covers its usage as well.

One can control the basic options for PETSc with the PROBLEM keyword: https://www.seamplex.com/feenox/doc/feenox-manual.html#problem This is useful. Please make this link more prominent as I did not notice this before. If you are also using a "prefix" for the solver options, then it will be helpful to mention this in the documentation as well.

I mention this link in the tutorial again. There are no prefixed PETSc objects.

gtheler commented 9 months ago

Dear all, I finished the heat conduction tutorial: https://seamplex.com/feenox/doc/tutorials/320-thermal/

I agree the documentation is not clear enough so I started to re-design the README to point different users, namely

  1. Academics with knowledge of both PDE mathematics and programming
  2. Advanced users with experience in Unix scripting

  3. Newbies

    I'll keep you posted.

gtheler commented 9 months ago

@gtheler did you implement the changes you hinted at to @vijaysm e.g. the thermal tutorials featuring non-linear applications? Are we waiting for you to implement some requested changes?

@Kevin-Mattheus-Moerman the thermal tutorial explaining all the combinations of linear/non-linear and steady/transient capabilities is ready:

https://seamplex.com/feenox/doc/tutorials/320-thermal/ https://seamplex.com/feenox/doc/tutorials/320-thermal/README.pdf

I'm working on re-designing the README and overall documentation. Suggestions are welcome.

gtheler commented 9 months ago

I suggest fixing the "documentation" link to point to a dedicated page about guidelines to contributing users. Do you support pull requests? How are reviews done for new changes before acceptance? Github workflows are not used for CI and so how do you decide if new changesets are valid? Some details regarding these aspects will be helpful if you intend to receive contributions from new users.

FYI @vijaysm , Github workflows are used for CI: https://github.com/seamplex/feenox/blob/main/.github/workflows/compile_and_test.yaml https://github.com/seamplex/feenox/actions

Kevin-Mattheus-Moerman commented 9 months ago

@gtheler great, thanks for working on these points. Let us know when we need to call the reviewers to get back to it.