Skip to content

Fix conda package build - #52

Merged
rykerfish merged 31 commits into
mainfrom
conda_fix
Jul 7, 2025
Merged

Fix conda package build#52
rykerfish merged 31 commits into
mainfrom
conda_fix

Conversation

@RaulPPelaez

@RaulPPelaez RaulPPelaez commented Jul 2, 2025

Copy link
Copy Markdown
Contributor

Some changes in the conda build infrastructure must have broken the conda build scripts. I noticed some incorrect things here and there that were probably the cause of it.
I also made it so that the conda package is built with a generic blas implementation, instead of relying on MKL. By doing it this way the resulting conda package is compatible with any BLAS, including MKL.

@github-actions

github-actions Bot commented Jul 2, 2025

Copy link
Copy Markdown

Linter reported no issues

All Python files are correctly formatted with Black.

@github-actions

github-actions Bot commented Jul 2, 2025

Copy link
Copy Markdown

Linter reported no issues

All C/C++ files are correctly formatted with clang-format.

@RaulPPelaez

Copy link
Copy Markdown
Contributor Author

@rykerfish please review and merge. Wait until the last CI ends just in case it fails.
I will manually trigger the conda upload CI once this is merge (0.9.0 has not made it to anaconda yet due to the issues fixed here)

@RaulPPelaez
RaulPPelaez requested a review from rykerfish July 2, 2025 12:41
@RaulPPelaez

Copy link
Copy Markdown
Contributor Author

The error comes from find_package(blas) linking against libopenblas instead of the generic symlink called just "libblas.so". When testing I am installing libblas from mkl to test that it is compatible and it fails.
Not sure what the best way to tackle this is. UAMMDSetup.cmake links agains BLAS::BLAS instead of just blas, which is probably tbe culprit here.

@RaulPPelaez

Copy link
Copy Markdown
Contributor Author

This will probably work, but the resulting package will be compatible only with openblas. The key is in the finc_package(BLAS) and lapack most probably, somehow instructing it to find libblas.so instead of libopenblas.so

@rykerfish

Copy link
Copy Markdown
Contributor

Whoops, hadn't seen your original comment before my next commit! I'll do a bit more research to see how we can make it link to something generic instead of openblas specifically

@rykerfish

Copy link
Copy Markdown
Contributor

@RaulPPelaez I'm pretty surprised that last one failed. In the logs, I can see it finds the correct library

2025-07-02T21:13:54.7951452Z -- Found BLAS: $PREFIX/lib/libblas.so
2025-07-02T21:13:54.7953077Z -- Found LAPACK: $PREFIX/lib/liblapack.so;$PREFIX/lib/libblas.so

but it still fails. Could this be coming in on the solver-level CMake files? Each one calls uammd_setup_target and I'm a little suspicious of either these lines (although I think BLA_VENDOR should already be set and it should find Generic again) or maybe these lines? Alternatively, do we need to add libblas to runtime dependencies? The only thing that makes me think that is some of the warnings in the github runner logs, so I suspect we should look into other things first.

@RaulPPelaez

Copy link
Copy Markdown
Contributor Author

Holly molly, FINALLY. TBH, I cannot explain exactly what the culprit was. Blas has a lot of idiosynchratic behaviors in conda-forge.

@RaulPPelaez

Copy link
Copy Markdown
Contributor Author

Please review again @rykerfish

@rykerfish rykerfish left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! I suspect part of our earlier problem was missing the correct BLAS libraries on the host side but that was hidden under the layer of us linking to the wrong BLAS binary. Nice fix!

@rykerfish
rykerfish merged commit 9c32369 into main Jul 7, 2025
5 checks passed
@rykerfish
rykerfish deleted the conda_fix branch July 7, 2025 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants