search

conda-forge / suitesparse-feedstock

A conda-smithy repository for suitesparse.
BSD 3-Clause "New" or "Revised" License
1 stars 16 forks source link

Update blas, openmp dependencies #97

Closed jeongseok-meta closed 1 week ago

jeongseok-meta commented 2 weeks ago

As blas-devel is not added as a run dependency, downstream packages see the following error due to missing blas-devel, which is needed by this package.

-- Found Dr.Jit: C:/Users/jeongseok/dev/tmp/test/.pixi/envs/default/Library/include (found version "0.4.6" )
CMake Error at .pixi/envs/default/Library/lib/cmake/Ceres/FindSuiteSparse.cmake:182 (message):
  Failed to find SuiteSparse - Did not find BLAS library (required for
  SuiteSparse).
Call Stack (most recent call first):
  .pixi/envs/default/Library/lib/cmake/Ceres/FindSuiteSparse.cmake:295 (suitesparse_report_not_found)
  C:/Program Files/CMake/share/cmake-3.23/Modules/CMakeFindDependencyMacro.cmake:47 (find_package)
  .pixi/envs/default/Library/lib/cmake/Ceres/CeresConfig.cmake:181 (find_dependency)
  C:/Program Files/CMake/share/cmake-3.23/Modules/CMakeFindDependencyMacro.cmake:47 (find_package)
  .pixi/envs/default/Library/lib/cmake/momentum/momentum-config.cmake:39 (find_dependency)
  CMakeLists.txt:5 (find_package)

The downstream package can resolve this by adding blas-devel explicitly, but it would be ideal to add it to the suitesparse package as it's a transitive dependency.

Checklist

conda-forge-admin commented 2 weeks ago

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe/meta.yaml) and found it was in an excellent condition.

jeongseok-meta commented 2 weeks ago

@conda-forge-admin, please rerender

minrk commented 2 weeks ago

I think probably blas-devel actually shouldn't be used here either. It looks like we should probably be setting -DBLAS_LIBRARIES=blas;cblas;lapack to directly link blas instead of -DBLA_PREFER_PKGCONFIG=ON. Do you want to try to tackle that, or would you like me to have a go? Looking at the link check output, I think we might also be missing an explicit dependency on openmp in host:

  host:
    - llvm-openmp  # [osx]
    - libgomp  # [linux]
jeongseok-meta commented 2 weeks ago

Thank you for taking a look into this issue and for your new findings! I'm happy to let you go ahead with the changes, which I greatly appreciate. Please let me know if there's anything else I can assist with.

minrk commented 2 weeks ago

@jeongseok-meta where are you seeing the missing blas error? Is it using a package at runtime, or is it during the build of something with conda-build? If it's conda-build, is it on conda-forge or built elsewhere? Does your package run cmake in user environments at runtime? I don't see these errors on conda-forge/momentum-feedstock, which makes me think it's elsewhere, and it would be helpful to know the circumstances.

Looking through some of the cmake files, I suspect that the changes here now, while I think they are correct for this repo, aren't going to fix your problem. The reason being that blas-devel is required for FindBLAS if netlib blas is not present. This is not a runtime requirement, but I think it does mean FindSuiteSparse will only succeed if netlib variants are present.

The problem also appears to not originate in suitesparse after all, but rather Ceres own custom FindSuiteSparse that's running into this. I don't know why Ceres isn't using direct find_package for the suitesparse components it uses (i.e. find_package(AMD), find_package(SQPR) etc.), which I think would work fine.

minrk commented 1 week ago

@jeongseok-meta I did some digging, and I believe what will actually fix your problem is https://github.com/conda-forge/ceres-solver-feedstock/pull/46 not this. I'm going to merge this as the right change, but I don't think there's anything in suitesparse to fix ceres looking for a bunch of things it doesn't need when it's trying to find suitesparse.

edit: fixed link

jeongseok-meta commented 1 week ago

Hi @minrk , thank you for putting in a lot of effort on this issue, and I apologize for not getting back to you earlier.

I see the missing blas error when running CMake with the Ceres dependency, similar to the test example you added (https://github.com/conda-forge/ceres-solver-feedstock/pull/46). Based on your research, it indeed seems an issue with CeresSolver, but not with the suitesparse package. The momentum-fedstock didn't have the error because blas-devel is explicitly added (https://github.com/conda-forge/momentum-feedstock/blob/4fbe0b7e465f1cf9f8ccdc9e02eae79a45071626/recipe/meta.yaml#L67), which shouldn't be necessary if there isn't a transitive dependency issue from CeresSolver, I believe.

It sounds like I need to test with (https://github.com/conda-forge/ceres-solver-feedstock/pull/46) once it's merged. Thank you again for your effort on this matter!