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.11k stars 256 forks source link

UMFPACK: Display timer function used by SuiteSparse_config in report #823

Closed mmuetzel closed 1 month ago

mmuetzel commented 1 month ago

UMFPACK is built without OpenMP. (It doesn't use it directly.) Hence, _OPENMP is not defined when the UMFPACK libraries are built.

Get the information about the used timer function directly from SuiteSparse_config.h instead.

Fixes #822.

I used the following reference for the "stringification" of the preprocessor macro: https://gcc.gnu.org/onlinedocs/cpp/Stringizing.html

mmuetzel commented 1 month ago

Oops. What I wrote is probably not entirely correct.

I was assuming that all libraries would be using SuiteSparse_tic, SuiteSparse_toc or SuiteSparse_time to measure time. However, it looks like some libraries (or demos) are using SUITESPARSE_TIME directly. For that to work, they need to build with OpenMP (if clock_gettime is not available).

Additionally, the proposed change doesn't do what I thought it would because SUITESPARSE_TIME is set to a value that depends on the library in which SuiteSparse_config.h is included (and not on how libsuitesparse_config itself was built).

Should SUITESPARSE_TIME really be used in libraries that depend on libsuitesparse_config? Should these libraries (and demos) better use SuiteSparse_time instead?

mmuetzel commented 1 month ago

I added a second commit that should be fixing the issues described in the previous comment.

mmuetzel commented 1 month ago

It looks like the changes from this PR triggered a test error on the runner using clang-cl. It is likely that there is an issue with the OpenMP configuration on that runner indeed. It is linking libraries that are using the Intel OpenMP implementation (the Intel MKL BLAS/LAPACK libraries) and libraries that are using the LLVM OpenMP implementation (the SuiteSparse libraries). That might also be an issue on the runners that are using MSVC cl. But it looks like they do not have a similar runtime check in the MSVC implementation of OpenMP.

I'll try to look into that soon. (But that will likely result in a different PR.)

mmuetzel commented 1 month ago

Converted to draft for now to wait for #824 and rebase.

mmuetzel commented 1 month ago

This seems to be working now.

It looks like GitHub is in the process of updating MSVC from 19.39 to 19.40 on their runners. And the CUDA version that we are installing isn't compatible with that newer version of MSVC. But that is unrelated to the changes proposed here.