search

DrTimothyAldenDavis / SuiteSparse

The official SuiteSparse library: a suite of sparse matrix algorithms authored or co-authored by Tim Davis, Texas A&M University.
https://people.engr.tamu.edu/davis/suitesparse.html
Other
1.17k stars 262 forks source link

GraphBLAS does not use OpenMP with 7.4.0.beta10 (macOS with OpenMP-capable compiler) #634

Closed szhorvat closed 10 months ago

szhorvat commented 10 months ago

Describe the bug

GraphBLAS does not use OpenMP, even if it is explicitly requested.

To Reproduce

Clone the repo, check out tag v7.4.0.beta10. Here's a terminal transcript. clang-mp-17 is Clang 17 from MacPorts, with OpenMP support (as Apple Clang doesn't do OpenMP). It does not matter if I leave off -DGRAPHBLAS_USE_OPENMP=ON -DSUITESPARSE_USE_OPENMP=ON, the outcome is the same.

~/R/SuiteSparse ((v7.4.0.beta10)|✔) $ cd GraphBLAS/build/
~/R/S/G/build ((v7.4.0.beta10)|✔) $ CC=clang-mp-17 CXX=clang++-mp-17 cmake .. -DGRAPHBLAS_USE_OPENMP=ON -DSUITESPARSE_USE_OPENMP=ON
-- The C compiler identification is Clang 17.0.6
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /opt/local/bin/clang-mp-17 - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Building SuiteSparse:GraphBLAS version: v8.3.1, date: Dec 30, 2023
-- GraphBLAS C API: v2.0, date: Nov 15, 2021
-- Source:           /Users/szhorvat/Repos/SuiteSparse/GraphBLAS 
-- Build:            /Users/szhorvat/Repos/SuiteSparse/GraphBLAS/build 
-- Install lib:      lib
-- Install include:  include/suitesparse
-- Install bin:      bin
-- Install pkg-file: lib
-- Install rpath:    $ORIGIN
-- Build   rpath:    
-- Build type:       Release 
-- Looking for a Fortran compiler
-- Looking for a Fortran compiler - /Users/szhorvat/bin/gfortran
-- The Fortran compiler identification is GNU 13.2.0
-- Checking whether Fortran compiler has -isysroot
-- Checking whether Fortran compiler has -isysroot - yes
-- Checking whether Fortran compiler supports OSX deployment target flag
-- Checking whether Fortran compiler supports OSX deployment target flag - yes
-- Detecting Fortran compiler ABI info
-- Detecting Fortran compiler ABI info - done
-- Check for working Fortran compiler: /Users/szhorvat/bin/gfortran - skipped
-- Fortran:          /Users/szhorvat/bin/gfortran
-- Looking for a CUDA compiler
-- Looking for a CUDA compiler - NOTFOUND
-- CUDA:             not found
-- CUDA:             not enabled
-- GraphBLAS CUDA JIT: disabled
-- GraphBLAS CPU JIT: enabled
-- Found OpenMP_C: -fopenmp=libomp  
-- Found OpenMP_Fortran: -fopenmp (found version "4.5") 
-- Found OpenMP: TRUE (found version "4.5")  
-- cpu_features (by google.com): enabled 
-- Building dynamic GraphBLAS library (no static)
-- Looking for dlfcn.h
-- Looking for dlfcn.h - found
-- cpu_feautures has dlfcn.h
-- Looking for getauxval
-- Looking for getauxval - not found
-- cpu_feautures doesn't have getauxval from sys/auxv.h
-- Performing Test TEST_FOR_STDATOMIC
-- Performing Test TEST_FOR_STDATOMIC - Success
-- C11 atomics: OK. -latomic not needed
-- CMAKE OpenMP libraries:    /opt/local/lib/libomp/libomp.dylib
-- CMAKE OpenMP include:      
-- CMAKE OpenMP C flags:      -fopenmp=libomp
-- CMAKE C flags:  -Wno-pointer-sign  -O3 -DNDEBUG 
-- Skipping the demos in GraphBLAS/Demo
-- ------------------------------------------------------------------------
-- JIT configuration:
-- ------------------------------------------------------------------------
-- JIT C compiler: /opt/local/bin/clang-mp-17
-- JIT C flags:     -Wno-pointer-sign  -O3 -DNDEBUG -fPIC  -arch arm64  -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.2.sdk 
-- JIT link flags:  -dynamiclib 
-- JIT lib prefix: lib
-- JIT lib suffix: .dylib
-- JIT obj suffix: .o
-- JIT cache:      /Users/szhorvat/.SuiteSparse/GrB8.3.1
-- JIT openmp inc: 
-- JIT openmp dirs 
-- JIT libraries:   -lm -ldl /opt/local/lib/libomp/libomp.dylib
-- JIT cmake libs: m;dl;/opt/local/lib/libomp/libomp.dylib
-- ------------------------------------------------------------------------
-- SuiteSparse CMAKE report for: graphblas
-- ------------------------------------------------------------------------
-- inside common SuiteSparse root:  OFF
-- install in SuiteSparse/lib and SuiteSparse/include: OFF
-- build type:           Release
-- BUILD_SHARED_LIBS:    ON
-- BUILD_STATIC_LIBS:    OFF
-- use OpenMP:           no 
-- C compiler:           /opt/local/bin/clang-mp-17 
-- C flags:               -Wno-pointer-sign  -O3 -DNDEBUG
-- C++ compiler:         
-- C++ flags:            
-- C Flags release:      -O3 -DNDEBUG 
-- C++ Flags release:     
-- Fortran compiler:     /Users/szhorvat/bin/gfortran 
-- compile definitions:  
-- ------------------------------------------------------------------------
-- Configuring done
-- Generating done
-- Build files have been written to: /Users/szhorvat/Repos/SuiteSparse/GraphBLAS/build

Expected behavior

Output should show use OpenMP: yes. Instead it shows:

-- use OpenMP:           no

Desktop (please complete the following information):

Additional context —

DrTimothyAldenDavis commented 10 months ago

Thanks for the update. I found that too and I'm in the process of fixing it. I think it's actually using OpenMP but the message is not correct. I have a few commits that fix this but not yet a beta11 because I still have to confirm that it's fixed.

DrTimothyAldenDavis commented 10 months ago

This should be fixed now in beta11, https://github.com/DrTimothyAldenDavis/SuiteSparse/releases/tag/v7.4.0.beta11

szhorvat commented 10 months ago

@DrTimothyAldenDavis Thanks for the fix! It's now compiling on my machine (it takes a while). While I'm waiting for it, a few questions about SuiteSparse in general:

DrTimothyAldenDavis commented 10 months ago
  • Which packages support OpenMP, and which ones do you recommend enabling it for? I understand that it is essential for GraphBLAS, and probably for LAGraph as well. I would only enable it for essential ones as there seem to be potentially bad interactions with OpenBLAS that I do not understand well.

Here are the packages that can use OpenMP directly (not counting the possible use in the BLAS / LAPACK):

Each of those packages has its own Package_USE_OPENMP flag (but I see one is missing in the top README.md):

Other packages might use OpenMP if the BLAS/LAPACK need it: UMFPACK, CHOLMOD, SPQR, and ParU.

  • At the moment, parallel build is disabled for SuiteSparse in MacPorts. Do you have any idea why this may have been done? I am re-enabling it an hoping nothing breaks.

No idea. Maybe because UMFPACK, etc use the BLAS and get performance that way. GraphBLAS and ParU are new.

  • MacPorts wants CMAKE_MACOSX_RPATH to be OFF. From what I'm told this is so that it can detect linking issues when a library is updated and its dependents need re-compiling. SuiteSparse has CMAKE_MACOSX_RPATH hard-coded to ON, even though as I understand this is a default for CMake 3.0 and late (reference). As far as I can tell, all SuiteSparse packages now require CMake 3.22, with the exception of CSparse which requires 3.13. Grepping the source-tree for cmake_minimum_required reveals lower version requirements only for some vendored libraries, such as CHOLMOD/SuiteSparse_metis or GraphBLAS/cpu_features. Would it be then possible to not hard-code CMAKE_MACOSX_RPATH so that it wouldn't need to be patched in MacPorts?

I'm not sure why this was hard coded. I think I had trouble getting my codes to compile on the Mac. If it's the default for 3.0 and later, then I can remove it, and then MacPorts and set it OFF as needed.

DrTimothyAldenDavis commented 10 months ago

See the dev2 branch for the update to the README, just now:

https://github.com/DrTimothyAldenDavis/SuiteSparse/commit/b4f5a78066e8edbb8f04095b654e605a5c9d28b8

szhorvat commented 10 months ago

Confirmed fixed, closing.

DrTimothyAldenDavis commented 10 months ago

See https://github.com/DrTimothyAldenDavis/SuiteSparse/pull/637 . I've also removed all instances of set ( CMAKE_MACOSX_RPATH TRUE ).

szhorvat commented 10 months ago

Sundials 5.8 (sundials5 port in MacPorts) failed to compile with 7.4.0 (it did with 7.3.1). It complains about a missing klu.h. I won't debug this today. I'll report back after I've figured out the cause.

DrTimothyAldenDavis commented 10 months ago

It might be looking in /usr/local/include. All the include files for SuiteSparse are in /usr/local/include/suitesparse. If sundials uses cmake, this should be found automatically via the *Config.cmake files. But if it doesn't, it would need to look at /usr/local/include/suitesparse.

szhorvat commented 10 months ago

Yes, that was the issue, and it's fixable by -DSUITESPARSE_INCLUDEDIR_POSTFIX="".

I tried to include ParU as well, but I'm getting:

CMake Error at CMakeLists.txt:233 (target_include_directories):
  Error evaluating generator expression:

    $<TARGET_PROPERTY:SuiteSparse::UMFPACK,INTERFACE_INCLUDE_DIRECTORIES>

  Target "SuiteSparse::UMFPACK" not found.

I have not investigated this yet. Before I do so, do you have any tips?

This is not a big deal as I might just not include ParU yet, since it doesn't have a stable API.

Here's a full output form MacPorts:

ParU installation ``` ---> Checksumming SuiteSparse-7.4.0.beta12.tar.gz ---> Extracting SuiteSparse_ParU ---> Extracting SuiteSparse-7.4.0.beta12.tar.gz Executing: cd "/opt/local/var/macports/build/_Users_szhorvat_macports-ports_math_SuiteSparse/SuiteSparse_ParU/work" && /usr/bin/gzip -dc '/opt/local/var/macports/distfiles/SuiteSparse/SuiteSparse-7.4.0.beta12.tar.gz' | /usr/bin/tar -xf - ---> Applying patches to SuiteSparse_ParU ---> Applying patch-KLU-Include-klu_version.h.diff Executing: cd "/opt/local/var/macports/build/_Users_szhorvat_macports-ports_math_SuiteSparse/SuiteSparse_ParU/work/SuiteSparse-7.4.0.beta12" && /usr/bin/patch -p0 < '/Users/szhorvat/macports-ports/math/SuiteSparse/files/patch-KLU-Include-klu_version.h.diff' patching file 'KLU/Include/klu_version.h' ---> Configuring SuiteSparse_ParU Executing: cd "/opt/local/var/macports/build/_Users_szhorvat_macports-ports_math_SuiteSparse/SuiteSparse_ParU/work/build" && /opt/local/bin/cmake -G "CodeBlocks - Unix Makefiles" -DCMAKE_BUILD_TYPE=MacPorts -DCMAKE_INSTALL_PREFIX="/opt/local" -DCMAKE_INSTALL_NAME_DIR="/opt/local/lib" -DCMAKE_SYSTEM_PREFIX_PATH="/opt/local;/usr" -DCMAKE_C_COMPILER="$CC" -DCMAKE_CXX_COMPILER="$CXX" -DCMAKE_OBJC_COMPILER="$CC" -DCMAKE_OBJCXX_COMPILER="$CXX" -DCMAKE_POLICY_DEFAULT_CMP0025=NEW -DCMAKE_POLICY_DEFAULT_CMP0060=NEW -DCMAKE_VERBOSE_MAKEFILE=ON -DCMAKE_COLOR_MAKEFILE=ON -DCMAKE_FIND_FRAMEWORK=LAST -DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DCMAKE_MAKE_PROGRAM=/usr/bin/make -DCMAKE_MODULE_PATH="/opt/local/share/cmake/Modules" -DCMAKE_PREFIX_PATH="/opt/local/share/cmake/Modules" -DCMAKE_BUILD_WITH_INSTALL_RPATH:BOOL=ON -DCMAKE_INSTALL_RPATH="/opt/local/lib" -Wno-dev -DBUILD_STATIC_LIBS=OFF -DPARU_USE_OPENMP=ON -DBLA_VENDOR=OpenBLAS -DCMAKE_OSX_ARCHITECTURES="arm64" -DCMAKE_OSX_DEPLOYMENT_TARGET="13.0" -DCMAKE_OSX_SYSROOT="/Library/Developer/CommandLineTools/SDKs/MacOSX13.sdk" /opt/local/var/macports/build/_Users_szhorvat_macports-ports_math_SuiteSparse/SuiteSparse_ParU/work/SuiteSparse-7.4.0.beta12/ParU -- Building PARU version: v0.1.0 (Dec 30, 2023) -- The C compiler identification is Clang 16.0.6 -- The CXX compiler identification is Clang 16.0.6 -- Detecting C compiler ABI info -- Detecting C compiler ABI info - done -- Check for working C compiler: /opt/local/bin/clang-mp-16 - skipped -- Detecting C compile features -- Detecting C compile features - done -- Detecting CXX compiler ABI info -- Detecting CXX compiler ABI info - done -- Check for working CXX compiler: /opt/local/bin/clang++-mp-16 - skipped -- Detecting CXX compile features -- Detecting CXX compile features - done -- Source: /opt/local/var/macports/build/_Users_szhorvat_macports-ports_math_SuiteSparse/SuiteSparse_ParU/work/SuiteSparse-7.4.0.beta12/ParU -- Build: /opt/local/var/macports/build/_Users_szhorvat_macports-ports_math_SuiteSparse/SuiteSparse_ParU/work/build -- Install lib: lib -- Install include: include/suitesparse -- Install bin: bin -- Install pkg-file: lib -- Install rpath: $ORIGIN;/opt/local/lib -- Build rpath: -- Build type: MacPorts -- Looking for a Fortran compiler -- Looking for a Fortran compiler - NOTFOUND -- Fortran: not available -- Looking for a CUDA compiler -- Looking for a CUDA compiler - NOTFOUND -- CUDA: not found -- CUDA: not enabled -- Found OpenMP_C: -fopenmp=libomp (found version "5.0") -- Found OpenMP_CXX: -fopenmp=libomp (found version "5.0") -- Found OpenMP: TRUE (found version "5.0") -- ParU has OpenMP: ON -- Could NOT find SuiteSparse_config (missing: SuiteSparse_config_DIR) -- SuiteSparse_config version: 7.4.0 -- SuiteSparse_config include: /opt/local/include/suitesparse -- SuiteSparse_config library: /opt/local/lib/libsuitesparseconfig.7.4.0.dylib -- SuiteSparse_config static: -- Could NOT find CHOLMOD (missing: CHOLMOD_DIR) -- AMD version: 3.3.0 -- AMD include: /opt/local/include/suitesparse -- AMD library: /opt/local/lib/libamd.3.3.0.dylib -- AMD static: -- COLAMD version: 3.3.0 -- COLAMD include: /opt/local/include/suitesparse -- COLAMD library: /opt/local/lib/libcolamd.3.3.0.dylib -- COLAMD static: -- CAMD version: 3.3.0 -- CAMD include: /opt/local/include/suitesparse -- CAMD library: /opt/local/lib/libcamd.3.3.0.dylib -- CAMD static: -- CCOLAMD version: 3.3.0 -- CCOLAMD include: /opt/local/include/suitesparse -- CCOLAMD library: /opt/local/lib/libccolamd.3.3.0.dylib -- CCOLAMD static: -- CHOLMOD version: 5.1.0 -- CHOLMOD include: /opt/local/include/suitesparse -- CHOLMOD library: /opt/local/lib/libcholmod.5.1.0.dylib -- CHOLMOD static: -- Could NOT find UMFPACK (missing: UMFPACK_DIR) -- Looking for 32-BLAS: OpenBLAS -- Looking for sgemm_ -- Looking for sgemm_ - found -- Found BLAS: /opt/local/lib/libopenblas.dylib -- Found OpenBLAS 32-bit BLAS -- Specific BLAS: OpenBLAS found: TRUE -- BLAS integer size: 4 -- OpenMP C++ libraries: /opt/local/lib/libomp/libomp.dylib -- OpenMP C++ include: -- OpenMP C++ flags: -fopenmp=libomp -- BLAS libraries: /opt/local/lib/libopenblas.dylib -- BLAS linker flags: -- Skipping the demos in ParU/Demo -- ------------------------------------------------------------------------ -- SuiteSparse CMAKE report for: paru -- ------------------------------------------------------------------------ -- inside common SuiteSparse root: OFF -- install in SuiteSparse/lib and SuiteSparse/include: OFF -- build type: MacPorts -- BUILD_SHARED_LIBS: ON -- BUILD_STATIC_LIBS: OFF -- C compiler: /opt/local/bin/clang-mp-16 -- C flags: -pipe -O3 -DNDEBUG -I/opt/local/include -isysroot/Library/Developer/CommandLineTools/SDKs/MacOSX13.sdk -- C++ compiler: /opt/local/bin/clang++-mp-16 -- C++ flags: -pipe -O3 -DNDEBUG -I/opt/local/include -stdlib=libc++ -isysroot/Library/Developer/CommandLineTools/SDKs/MacOSX13.sdk -- C Flags release: -O3 -DNDEBUG -- C++ Flags release: -O3 -DNDEBUG -- Fortran compiler: none -- compile definitions: BLAS_OpenBLAS -- BLAS integer: int32_t -- ------------------------------------------------------------------------ -- Configuring done CMake Error at CMakeLists.txt:233 (target_include_directories): Error evaluating generator expression: $ Target "SuiteSparse::UMFPACK" not found. CMake Error at CMakeLists.txt:233 (target_include_directories): Error evaluating generator expression: $ Target "SuiteSparse::UMFPACK" not found. CMake Error at CMakeLists.txt:233 (target_include_directories): Error evaluating generator expression: $ Target "SuiteSparse::UMFPACK" not found. CMake Error at CMakeLists.txt:233 (target_include_directories): Error evaluating generator expression: $ Target "SuiteSparse::UMFPACK" not found. CMake Error at CMakeLists.txt:233 (target_include_directories): Error evaluating generator expression: $ Target "SuiteSparse::UMFPACK" not found. CMake Error at CMakeLists.txt:232 (target_link_libraries): Target "ParU" links to: SuiteSparse::UMFPACK but the target was not found. Possible reasons include: * There is a typo in the target name. * A find_package call is missing for an IMPORTED target. * An ALIAS target is missing. -- Generating done CMake Warning: Manually-specified variables were not used by the project: CMAKE_OBJCXX_COMPILER CMAKE_OBJC_COMPILER CMAKE_POLICY_DEFAULT_CMP0025 CMAKE_POLICY_DEFAULT_CMP0060 CMake Generate step failed. Build files cannot be regenerated correctly. Command failed: cd "/opt/local/var/macports/build/_Users_szhorvat_macports-ports_math_SuiteSparse/SuiteSparse_ParU/work/build" && /opt/local/bin/cmake -G "CodeBlocks - Unix Makefiles" -DCMAKE_BUILD_TYPE=MacPorts -DCMAKE_INSTALL_PREFIX="/opt/local" -DCMAKE_INSTALL_NAME_DIR="/opt/local/lib" -DCMAKE_SYSTEM_PREFIX_PATH="/opt/local;/usr" -DCMAKE_C_COMPILER="$CC" -DCMAKE_CXX_COMPILER="$CXX" -DCMAKE_OBJC_COMPILER="$CC" -DCMAKE_OBJCXX_COMPILER="$CXX" -DCMAKE_POLICY_DEFAULT_CMP0025=NEW -DCMAKE_POLICY_DEFAULT_CMP0060=NEW -DCMAKE_VERBOSE_MAKEFILE=ON -DCMAKE_COLOR_MAKEFILE=ON -DCMAKE_FIND_FRAMEWORK=LAST -DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DCMAKE_MAKE_PROGRAM=/usr/bin/make -DCMAKE_MODULE_PATH="/opt/local/share/cmake/Modules" -DCMAKE_PREFIX_PATH="/opt/local/share/cmake/Modules" -DCMAKE_BUILD_WITH_INSTALL_RPATH:BOOL=ON -DCMAKE_INSTALL_RPATH="/opt/local/lib" -Wno-dev -DBUILD_STATIC_LIBS=OFF -DPARU_USE_OPENMP=ON -DBLA_VENDOR=OpenBLAS -DCMAKE_OSX_ARCHITECTURES="arm64" -DCMAKE_OSX_DEPLOYMENT_TARGET="13.0" -DCMAKE_OSX_SYSROOT="/Library/Developer/CommandLineTools/SDKs/MacOSX13.sdk" /opt/local/var/macports/build/_Users_szhorvat_macports-ports_math_SuiteSparse/SuiteSparse_ParU/work/SuiteSparse-7.4.0.beta12/ParU Exit code: 1 Error: Failed to configure SuiteSparse_ParU: consult /opt/local/var/macports/build/_Users_szhorvat_macports-ports_math_SuiteSparse/SuiteSparse_ParU/work/build/CMakeFiles/CMakeError.log Error: Failed to configure SuiteSparse_ParU: configure failure: command execution failed Error: See /opt/local/var/macports/logs/_Users_szhorvat_macports-ports_math_SuiteSparse/SuiteSparse_ParU/main.log for details. Error: Follow https://guide.macports.org/#project.tickets if you believe there is a bug. Error: Processing of port SuiteSparse_ParU failed ```
szhorvat commented 10 months ago

Finally, I wanted to draw your attention to this patch that exists in MacPorts. According to the comments, this is necessary on very old OS X systems that run on PPC, so it's probably not worth bothering with it. I certainly can't (and won't) test on such old systems. But I thought I'd let you know just in case. https://github.com/macports/macports-ports/blob/master/math/SuiteSparse/files/patch-KLU-Include-klu_version.h.diff

DrTimothyAldenDavis commented 10 months ago

Ack. Hard to believe that Apple wrote a math.h that defined terms "Real" and "Imag". Those aren't in any C standard and exposing such generic terms to an end user code (like KLU) is a bad idea on their part. At least they don't do that anymore.

I could change that but I'm worried I might break something else. It could be that an end user sees those structs and changing them would break their code.

Since it's an obscure case on MacPorts I would prefer not to change it.

Not sure why umfpack is not found. It should be built just before ParU, right? I'm away from the keyboard just now, on my phone, but I'll take a closer look later.

szhorvat commented 10 months ago

@DrTimothyAldenDavis The ParU readme says,

ParU: is a set of routings

Should this not be "routines" instead of "routings"?

DrTimothyAldenDavis commented 10 months ago

Yes. The that's a typo. I'll fix it when I get back to the keyboard

DrTimothyAldenDavis commented 10 months ago

See https://github.com/DrTimothyAldenDavis/SuiteSparse/pull/640