Electrostatics / FETK

Fork of the Finite Element ToolKit from fetk.org.
Other
4 stars 4 forks source link

Suggestion: Preferentially use SuperLU installed by Homebrew/Linuxbrew #62

Closed YoshitakaMo closed 2 years ago

YoshitakaMo commented 2 years ago

Dear developers of FETK and APBS,

I'm very happy to hear that APBS v3.4.0 has been released recently. The manual installation from the source worked correctly according to the guide of https://apbs.readthedocs.io/en/latest/getting/source.html. However, there's one thing I'd like to see improved; If SuperLU is already installed by Homebrew/Linuxbrew, FETK should use it preferentially. Now I'm making a Homebrew Formula of APBS 3.4.0 to install the latest APBS in the easiest way. See my Homebrew formula (WIP) https://gist.github.com/YoshitakaMo/8cc5389ef55d820f688d4c6b1200ab9c . You can install APBS with Homebrew/Linuxbrew:

wget https://gist.github.com/YoshitakaMo/8cc5389ef55d820f688d4c6b1200ab9c
brew install ./apbs.rb --build-from-source --verbose --debug --keep-tmp

I tested it on both Intel and M1 mac (not on Linux yet), and the installation was successfully finished. But an error was caused at brew link step:

Error: The `brew link` step did not complete successfully
The formula built, but is not symlinked into /opt/homebrew
Could not symlink lib/cmake/superlu/superluConfig.cmake
Target /opt/homebrew/lib/cmake/superlu/superluConfig.cmake
is a symlink belonging to superlu. You can unlink it:
  brew unlink superlu

To force the link and overwrite all conflicting files:
  brew link --overwrite apbs

To list all files that would be deleted:
  brew link --overwrite --dry-run apbs

Possible conflicting files are:
/opt/homebrew/lib/cmake/superlu/superluConfig.cmake -> /opt/homebrew/Cellar/superlu/5.3.0/lib/cmake/superlu/superluConfig.cmake
/opt/homebrew/lib/cmake/superlu/superluConfigVersion.cmake -> /opt/homebrew/Cellar/superlu/5.3.0/lib/cmake/superlu/superluConfigVersion.cmake
/opt/homebrew/lib/cmake/superlu/superluTargets.cmake -> /opt/homebrew/Cellar/superlu/5.3.0/lib/cmake/superlu/superluTargets.cmake
/opt/homebrew/lib/pkgconfig/superlu.pc -> /opt/homebrew/Cellar/superlu/5.3.0/lib/pkgconfig/superlu.pc
==> Summary
🍺  /opt/homebrew/Cellar/apbs/3.4.0: 757 files, 108.2MB, built in 1 minute 7 seconds

I've found that this issue is caused by an additional build and installation of SuperLU described in FETK/punc/CMakeLists.txt. So, FETK's punc and APBS finally build lib/cmake/superlu/*.cmake files and install them into the Homebrew directory ( ....../Cellar/apbs/3.4.0/lib/cmake/superlu ).

This duplicate installation of SuperLU is suppressed when the FETK/punc/CMakeLists.txt is modified as follows:

# SuperLU library
# We only build SuperLU from its git repo
+ find_package( SUPERLU )
+ if(NOT SUPERLU_FOUND)
    set(BUILD_SUPERLU ON)
+     message(STATUS "Will build SUPERLU")
+ else()
+     set(BUILD_SUPERLU OFF)
+     message(STATUS "Will not build SUPERLU")
+ endif()

I am not completely sure yet if this modification will work correctly on all Intel/M1 Mac and Linux OSs, but I believe this change will greatly contribute to an easier installation of APBS using Homebrew. I hope you will consider it.

sobolevnrm commented 2 years ago

Hello -- thank you for this feedback. We will take your suggestion into consideration; however, we've tried hard to avoid significant dependencies on external repositories. We have experienced problems with those types of dependencies in the past.

Thanks again,

Nathan

intendo commented 2 years ago

@nsoblath we could look at this when/if we try to get a multithreaded version of APBS built.

sagitter commented 2 years ago

Hi all.

I'm experiencing same approach in Fedora: building FETK (that it's a new dependency of APBS) against system libraries (FlexiBLAS, Maloc, SuperLU, libarpack) is a little more complicated. Mainly, FETK provides maloc-1.21 against the 1.5 distributed in Fedora, shall i think that FETK works with the 1.21 only? If APBS will be released including FETK (https://github.com/Electrostatics/apbs/issues/163), will it have the own dependent external bundled libraries?

nsoblath commented 2 years ago

I think it certainly makes sense to allow the PUNC module to be a little more flexible in how it deals with some dependencies. In particular, the most straightforward to improve would be:

@intendo, I suggest that we include these in our next round of updates.

Regarding the other packages mentioned by @sagitter, PUNC should already check for libarpack and only build it if it's not there. And as for Maloc, for now I think we're should stick with the version that's built into FETK. If someone wants to provide a change that allows an external Maloc dependency, we could certainly integrate that into the repo. I don't think it'd require extra support in the future.

intendo commented 2 years ago

@nsoblath if these changes are simple then yes. Otherwise, we should make it a lower priority.

drew-parsons commented 2 years ago

This issue affects the Debian linux packaging of APBS also. Previously debian prepared APBS without FETK but now (APBS v3.4) it's mandatory to use FETK. We'd prefer to use the system installed SuperLU.

I was able to build the punc components of FETK by switching BUILD_SUPERLU to OFF, though I'm not sure if SUPERLU_LIBRARIES (define during the cmake run) is getting through. In any case the build stalls not in punc but in bam, via zslu.c which includes slu_ddefs.h. SUPERLU_INCLUDES (defined when cmake was run) was delivered to punc but not to bam, so the build halts with

[ 62%] Building C object _deps/fetk-build/punc/src/CMakeFiles/punc.dir/base/punc_base.c.o
cd /build/apbs/obj-x86_64-linux-gnu/_deps/fetk-build/punc/src && /usr/bin/cc -Dpunc_EXPORTS -I/build/apbs/debian/external_deps/fetk/punc/../maloc/src/base -I/build/apbs/debian/external_deps/fetk/punc/../maloc/src/psh -I/build/apbs/debian/external_deps/fetk/punc/../maloc/src/vsh -I/ build/apbs/debian/external_deps/fetk/punc/../maloc/src/vsys -I/build/apbs/debian/external_deps/fetk/punc/src/vf2c -I/build/apbs/debian/external_deps/fetk/punc/src/vf2c/punc -I/build/apbs/debian/external_deps/fetk/punc/src/cgcode -I/build/apbs/debian/external_deps/fetk/punc/src/pmg -I/usr/include/superlu -I/build/apbs/obj-x86_64-linux-gnu/_deps/fetk-build/punc/src -I/build/apbs/debian/external_deps/fetk/punc/src -I/SRC -I"/build/apbs/debian/external_deps/fetk/punc/src/\$<INSTALL_INTERFACE:include/punc" -I"/build/apbs/debian/external_deps/fetk/punc/src/\$<INSTALL_INTERFACE:include/punc/punc" -g -O2 -ffile-prefix-map=/build/apbs=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -fcommon -Wdate-time -D_FORTIFY_SOURCE=2 -O2 -g -DNDEBUG -fPIC -fPIC -MD -MT _deps/fetk-build/punc/src/CMakeFiles/punc.dir/base/punc_base.c.o -MF CMakeFiles/punc.dir/base/punc_base.c.o.d -o CMakeFiles/punc.dir/base/punc_base.c.o -c /build/apbs/debian/external_deps/fetk/punc/src/base/punc_base.c
[ 62%] Building C object _deps/fetk-build/gamer/src/CMakeFiles/gamer.dir/triangle/triangle.c.o
[ 63%] Building CXX object _deps/fetk-build/gamer/src/CMakeFiles/gamer.dir/tetgen/predicates.cpp.o
...
[ 69%] Building C object _deps/fetk-build/mc/src/CMakeFiles/mc.dir/bam/zslu.c.o
cd /build/apbs/obj-x86_64-linux-gnu/_deps/fetk-build/mc/src && /usr/bin/cc -Dmc_EXPORTS -I/usr/include/suitesparse -I/build/apbs/debian/external_deps/fetk/mc/../maloc/src/base -I/build/apbs/debian/external_deps/fetk/mc/../maloc/src/psh -I/build/apbs/debian/external_deps/fetk/mc/../maloc/src/vsh -I/build/apbs/debian/external_deps/fetk/mc/../maloc/src/vsys -I/build/apbs/debian/external_deps/fetk/mc/../punc/src/base -I/build/apbs/obj-x86_64-linux-gnu/_deps/fetk-build/mc/src -I/build/apbs/debian/external_deps/fetk/mc/src/base -I/build/apbs/debian/external_deps/fetk/mc/src/bam -I/build/apbs/debian/external_deps/fetk/mc/src/whb -I/build/apbs/debian/external_deps/fetk/mc/src/aprx -I/build/apbs/debian/external_deps/fetk/mc/src/gem -I/build/apbs/debian/external_deps/fetk/mc/src/pde -I/build/apbs/debian/external_deps/fetk/mc/src/nam -I/build/apbs/debian/external_deps/fetk/mc/src/dyn -I/build/apbs/debian/external_deps/fetk/mc/src/mcsh -I/SRC -I"/build/apbs/debian/external_deps/fetk/punc/src/\$<INSTALL_INTERFACE:include/punc" -I"/build/apbs/debian/external_deps/fetk/punc/src/\$<INSTALL_INTERFACE:include/punc/punc" -g -O2 -ffile-prefix-map=/build/apbs=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -fcommon -Wdate-time -D_FORTIFY_SOURCE=2 -O2 -g -DNDEBUG -fPIC -fPIC -MD -MT _deps/fetk-build/mc/src/CMakeFiles/mc.dir/bam/zslu.c.o -MF CMakeFiles/mc.dir/bam/zslu.c.o.d -o CMakeFiles/mc.dir/bam/zslu.c.o -c /build/apbs/debian/external_deps/fetk/mc/src/bam/zslu.c
/build/apbs/debian/external_deps/fetk/mc/src/bam/zslu.c:64:10: fatal error: slu_ddefs.h: No such file or directory
   64 | #include "slu_ddefs.h"
      |          ^~~~~~~~~~~~~
compilation terminated.

since -I/usr/include/superlu, defined in SUPERLU_INCLUDES, didn't get added to bam (it did get added to punc).

As far as BLAS goes, the situation with debian is that we build against the generic libblas.so, and manage that using alternative symlinks to specific optimized implementations. So the system administrator can choose openblas, atlas, blis, intelmkl. That is, it would be helpful for FETK to use the system installed BLAS. But the issue currently blocking my build is SuperLU.

SuperLU does not change rapidly. Its API has been stable for years. There should be no reason to not use system installed libraries.

nsoblath commented 2 years ago

Status update (intended to be posted 4/20/22): With insight from the above comments, I have a build that gets to the point of the error finding slu_ddefs.h. The solution has to come from each module of FETK somehow exporting include directories that are necessary to use the module. The best way to do this is probably to use the techniques built into "modern" CMake for having include directories saved as properties of targets. This could involve some significant modifications to the build of FETK, but it's probably the cleanest solution. The alternative is using variables that are made available to dependent parts of the build.

nsoblath commented 2 years ago

Update: I've implemented the solution I suggested above, and everything seems to be working. I've demonstrated the build based on a pre-installed version of SuperLU using the Docker build. We'll push out a new release soon, but for now the working version can be found in branch nsoblath/issue_62. This'll be incorporated into APBS via Electrostatics/apbs#226.