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.14k stars 258 forks source link

cmake cache variables: Package_USE_OPENMP and SUITESPARSE_USE_OPENMP #742

Closed DrTimothyAldenDavis closed 6 months ago

DrTimothyAldenDavis commented 7 months ago

The way the cmake cache is manipulated for implementing the Package_USE_OPENMP, SUITESPARSE_USE_OPENMP, and Package_HAS_OPENMP flags in cmake (for a given Package: CHOLMOD, GraphBLAS, etc) is asymmetric, and is affecting how the use of OpenMP is controlled.

Perhaps the following cmake lines should simply be removed?

https://github.com/DrTimothyAldenDavis/SuiteSparse/blob/fcfefe63fb33a623fc66381832bdba79d6871d1b/CHOLMOD/CMakeLists.txt#L87-L89

https://github.com/DrTimothyAldenDavis/SuiteSparse/blob/fcfefe63fb33a623fc66381832bdba79d6871d1b/GraphBLAS/CMakeLists.txt#L104-L106

https://github.com/DrTimothyAldenDavis/SuiteSparse/blob/fcfefe63fb33a623fc66381832bdba79d6871d1b/GraphBLAS/GraphBLAS/CMakeLists.txt#L68-L70

https://github.com/DrTimothyAldenDavis/SuiteSparse/blob/fcfefe63fb33a623fc66381832bdba79d6871d1b/LAGraph/CMakeLists.txt#L165-L167

https://github.com/DrTimothyAldenDavis/SuiteSparse/blob/fcfefe63fb33a623fc66381832bdba79d6871d1b/ParU/CMakeLists.txt#L58-L60

https://github.com/DrTimothyAldenDavis/SuiteSparse/blob/fcfefe63fb33a623fc66381832bdba79d6871d1b/SuiteSparse_config/CMakeLists.txt#L74-L76

@mmuetzel : would this help resolve the issues raised in the discussion at https://github.com/DrTimothyAldenDavis/SuiteSparse/discussions/603#discussioncomment-8133399 regarding the changes I made in https://github.com/DrTimothyAldenDavis/SuiteSparse/commit/038bca2b44bb5cf0e58f9373c71a27feb4f35845 ?

mmuetzel commented 7 months ago

IIRC, that would be a preparatory part for the necessary changes. The "actual" fix can then be made somewhere further down in the build rules.

I can try to have a look.

DrTimothyAldenDavis commented 7 months ago

OK. I'll go ahead and make these changes.

mmuetzel commented 7 months ago

Thank you for changing that. That probably still doesn't fix the potential inconsistent linkage to the OpenMP target though. I probably won't have time to look into it until tomorrow. Do you have a date in mind for releasing the next version?

DrTimothyAldenDavis commented 7 months ago

Given the severity of the ABI problem for Debian (they've already applied the patch), I'd like to release SuiteSparse 7.5.2 as soon as possible.

Can we fix the OpenMP issue later? I could release a SuiteSparse 7.5.3 shortly after 7.5.2 for example, like next week, or whenever you like, to fix this OpenMP issue.

mmuetzel commented 6 months ago

I believe this is fixed in SuiteSparse 7.6.0.