search

MatMechLab / AsFem

Advanced Simulation kit based on Finite Element Method (AsFem)
https://matmechlab.github.io/AsFem
GNU General Public License v3.0
180 stars 53 forks source link

AsFEM installation on OSX #66

Closed vijaysm closed 2 years ago

vijaysm commented 2 years ago

While performing the review for the JOSS paper submission, I started down the path of installing AsFEM on osx. There are several comments from this experiment.

  1. If OSX is not supported, mention it explicitly in the documentation. You should list the platforms that are supported clearly and what compilers you expect the build to work without any user tweaks.
  2. -fopenmp is enabled by default and added to the build flags. This is unacceptable. This should be exposed as a user configurable option and the actual flag should depend on the compiler being used for the build.
  3. On OSX, the library is explicitly trying to link with libpetsc.so since this is hardcoded in the configure CMakeLists.txt. What happens when PETSc is configured as a static library only? How about a shared library in OSX that generates libpetsc.dylib?
  4. How do I run the tests when configuring out-of-source? This is the standard approach for CMake builds and ctest or make test does not produce any output. I haven't yet re-run in-source as I usually do not prefer this mode. However, if this is the recommended approach for AsFEM, please state this in the documentation.
vijaysm commented 2 years ago

Note: I will once again pursue installation on a linux workstation as well and will create a separate issue if I have other concerns.

yangbai90 commented 2 years ago

Dear @vijaysm , thanks for the feedback, I have revised the CMakeList file, now it should not be limited by libpetsc.so file. For test, we currently do not have a test from cmake side. But we have a 'AutoTest.py' script, one can run the test via: './scripts/AutoTest.py'.

vijaysm commented 2 years ago

It is standard practice to expose some regression tests to verify the consistency of the build. If you can expose the call to ./scripts/AutoTest.py with the right params automatically through a Makefile/CMake target, that would satisfy my requirements.

yangbai90 commented 2 years ago

Hi @vijaysm , I add the make test in this commit 2feb418247d6d5bbb54f3d67f6507afa0b8b3b29, after the make step, the make test will automatically call the AutoTest.py script for auto test.

vijaysm commented 2 years ago

If possible, please fix these warnings. It is not absolutely necessary for the review, but I usually like clean builds on default settings.

In file included from /Users/mahadevan/Code/AsFem/include/BCSystem/BCSystem.h:37:
In file included from /Users/mahadevan/Code/AsFem/include/FE/FE.h:22:
/Users/mahadevan/Code/AsFem/include/FE/QPoint.h:86:5: warning: non-void function does not return a value in all control paths [-Wreturn-type]
    }
    ^
/Users/mahadevan/Code/AsFem/include/FE/Lagrange3DShapeFun.h:73:12: warning: private field '_tol' is not used [-Wunused-private-field]
    double _tol=1.0e-15; /**< tolerance for error check*/
           ^
/Users/mahadevan/Code/AsFem/include/FE/Lagrange3DShapeFun.h:74:12: warning: private field '_val' is not used [-Wunused-private-field]
    double _val;/**< temporary variable*/
           ^
yangbai90 commented 2 years ago

Dear @vijaysm , many thanks for the meaningful suggestions.

Currently, I'm too busy with other stuffs, therefore, I'd like to cancel the submission to JOSS.

Thanks again for your helpful suggestion, the code has been reconstructed, therefore, I'll close this comment.

Best, Yang