build(wheels): source arpack/openblas/boost from conda-forge (Linux + macOS)#1057
build(wheels): source arpack/openblas/boost from conda-forge (Linux + macOS)#1057IvanaGyro wants to merge 10 commits into
Conversation
…acOS The Linux wheel build installed boost/openblas/arpack from EPEL via dnf (manylinux_2_28) and macOS installed them from Homebrew (cibuildwheel_before_all_macos.sh). Two independent problems followed from that: * EPEL's arpack-devel links whichever OpenBLAS dnf resolves, which is a pthreads-threaded build with its own thread pool, independent of the OpenMP runtime HPTT and cytnx's own OpenMP code use (CytnxBKNDCMakeLists.cmake's `-fopenmp`). A BLAS call made from inside an OpenMP region oversubscribes the CPU as a result. The same applies to Homebrew's OpenBLAS bottle on macOS. * Homebrew's bottled dylibs (arpack/boost/openblas/libomp) encode whatever macOS deployment target Homebrew itself currently targets for that formula, which trends upward release over release -- Cytnx's actual minimum supported macOS version rises as a side effect of Homebrew's own build policy rather than a decision made here. Both scripts now provision a self-contained micromamba environment from conda-forge instead: `openblas=*=*openmp*` selects the OpenMP-threaded OpenBLAS build (verified directly: its ELF NEEDED entry is libgomp.so.1, and the "openmp" conda-forge build variant -- via the _openmp_mutex coordination package -- routes that soname through LLVM's libomp, which was confirmed to export the full GOMP_1.0-5.0.1 versioned symbol set GCC's -fopenmp code needs, so cytnx's own OpenMP code and OpenBLAS end up sharing one thread pool rather than two); `arpack=*=nompi*` avoids vendoring an MPI runtime; `liblapacke`/`libboost-headers` replace the corresponding EPEL/Homebrew packages. ci-cmake_tests.yml's own mamba-based CPU test environment already relies on this same conda-forge OpenMP runtime (_openmp_mutex=*_llvm), so this is proven, not speculative. On macOS, conda-forge's clang_osx toolchain targets a pinned, deliberately old SDK, so its packages carry a controlled, low minos regardless of the CI runner's own OS version -- the existing dylib-minos-probing logic in cibuildwheel_before_all_macos.sh is kept unchanged and simply repointed at the new conda-forge prefix, so MACOSX_DEPLOYMENT_TARGET keeps tracking whatever floor the actual vendored dylibs require instead of a value guessed in advance. Verified directly against this exact package set (CMAKE_PREFIX_PATH pointed at a real micromamba-created environment, no docker/manylinux access in this environment): find_library(arpack), find_package(BLAS BLA_VENDOR OpenBLAS), find_package(LAPACK), find_package(LAPACKE), and find_package(Boost) (forcing MODULE mode, since the manylinux image has no competing system Boost config) all resolve correctly; a compiled program linking arpack+LAPACKE+Boost+OpenMP builds and runs correctly, including from inside an OpenMP parallel region. The macOS path itself could not be executed in this environment (no Mac available) and relies on the Linux-side verification plus reusing the existing, already-proven dylib-probing mechanism. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
_openmp_mutex=*=*_llvm was already pinned so the environment settles on a single OpenMP runtime, but blas=*=openblas on its own doesn't guarantee the solver picks OpenBLAS's OpenMP-threaded build variant over its pthreads-threaded one -- both are compatible with an environment where _openmp_mutex=llvm is present, since a pthreads-threaded OpenBLAS doesn't use OpenMP at all. Pin libopenblas=*=*openmp* explicitly, matching the wheel build (tools/cibuildwheel_before_all.sh): HPTT and cytnx's own code use OpenMP (-fopenmp), and an OpenBLAS build with its own independent thread pool oversubscribes the CPU when a BLAS call originates from inside an OpenMP region. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request transitions the CI build wheel process to use conda-forge (via micromamba) instead of Homebrew (on macOS) and dnf (on Linux manylinux) for installing dependencies like OpenBLAS (with OpenMP support), ARPACK, and Boost headers. This resolves issues with CPU oversubscription due to thread pool mismatches and stabilizes the minimum supported macOS version. The review feedback suggests improving the robustness of the installation scripts by explicitly installing bzip2 to ensure successful decompression of the micromamba archive, and adding the -f / --fail flag to curl commands to prevent silent failures on HTTP errors.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Three real bugs surfaced by this PR's own CI matrix:
* Linux: the micromamba download URL hardcoded linux-64 regardless of
the runner's actual architecture. On ubuntu-24.04-arm this fetched
an x86_64 binary, which failed with "Exec format error" the moment
before-all tried to run it. Detect the architecture the same way
the macOS script already does for osx-64/osx-arm64.
* macOS: the conda-forge deps prefix lived at ${PWD}/.cytnx-deps,
inside the checkout. CMake refuses to export an
INTERFACE_INCLUDE_DIRECTORIES entry that lives inside the source
directory ("which is prefixed in the source directory") -- a hard
configure-time error, hit here because the cytnx target exports that
property for downstream find_package(Cytnx) consumers (see
ci-downstream-find-package.yml). Move the prefix (and micromamba's
own root) to $TMPDIR, matching how the Linux script already used
/opt/cytnx-deps, outside the checkout by construction.
Also addresses two Gemini review suggestions: install bzip2 explicitly
on the dnf path (tar -xj needs it to decompress the micromamba
archive, and manylinux images don't guarantee it's present), and add
curl -f to both download commands so an HTTP error page can't get
silently piped into tar instead of failing the step outright.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
CI showed ccache (and, by the same mechanism, everything else under
the conda-forge prefix) unreachable on macOS: cibuildwheel's
[tool.cibuildwheel.macos].environment table evaluator does not expand
${VAR:-default}-style parameter substitution the way a real shell
does -- only plain $VAR references and $(command) substitutions are
honored, so CMAKE_PREFIX_PATH/PATH never resolved to the directory
tools/cibuildwheel_before_all_macos.sh actually created.
Write the resolved prefix to .cibw_macos_deps_prefix.txt in before-all
and read it back via $(cat ...), the same pattern
MACOSX_DEPLOYMENT_TARGET already uses successfully on the line right
below it.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
cibuildwheel evaluates [tool.cibuildwheel.macos].environment as part of setting up before-all's own subprocess environment, before before-all has run and created .cibw_macos_deps_prefix.txt at all. A bare `cat` on that not-yet-existing file exits non-zero and cibuildwheel treats that as a hard failure of the before-all step itself, never reaching the script that would have created the file. Read the file the same way MACOSX_DEPLOYMENT_TARGET already does on the line below -- a Path.exists() check with a fallback value -- so the one evaluation that happens before before-all runs degrades to a placeholder instead of aborting. Every evaluation after before-all has actually run (the build and test steps) sees the real file and resolves the real prefix. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1057 +/- ##
=======================================
Coverage 60.49% 60.49%
=======================================
Files 229 229
Lines 31873 31873
Branches 71 71
=======================================
Hits 19282 19282
Misses 12570 12570
Partials 21 21
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
Alpine's openblas-dev, like EPEL's on manylinux, is a pthreads-threaded build, and conda-forge (the fix used for manylinux and macOS) publishes no musl builds to swap in as an alternative. Build OpenBLAS v0.3.33 from source with USE_OPENMP=1 instead, and replace Alpine's copy with it in place. Verified directly on this host: openblas_get_num_threads() on an OpenMP-built OpenBLAS respects OMP_NUM_THREADS and ignores OPENBLAS_NUM_THREADS entirely (confirmed empirically -- setting only OPENBLAS_NUM_THREADS left the thread count at its OMP-derived default; setting both, OMP_NUM_THREADS won), matching OpenBLAS's own documented runtime-variable precedence for OpenMP builds. arpack's own NEEDED entry names the exact soname it was linked against (Alpine's OpenBLAS ABI name); discovered from the installed libarpack.so directly via readelf rather than hardcoded, so this keeps working if Alpine changes that soname. patchelf sets the built OpenBLAS's soname to match, and the replacement lands at the default system library paths (/usr/lib), so no CMAKE_PREFIX_PATH override is needed for musllinux. Not verified end-to-end in this environment (no musllinux/docker access in this sandbox) -- relying on this PR's own musllinux BuildWheel CI legs for that signal. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The deps prefix only needed to live outside the checkout (see the
prior commit fixing the same requirement), not to vary at all --
deriving it from $TMPDIR needed a file-based round-trip workaround
(cibuildwheel's environment-table evaluator doesn't expand
${VAR:-default} parameter substitution) purely to communicate a value
that could just as well be a fixed literal. Use /tmp/cytnx-deps
directly: pyproject.toml's CMAKE_PREFIX_PATH/PATH reference it as a
plain string, no discovery step needed.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1d4fed9e60
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # merely compatible with, but not actually using, that runtime. | ||
| run: | | ||
| mamba install _openmp_mutex=*=*_llvm cmake make boost git compilers blas=*=openblas liblapacke arpack gtest graphviz | ||
| mamba install _openmp_mutex=*=*_llvm cmake make boost git compilers "blas=*=openblas" "libopenblas=*=*openmp*" liblapacke arpack gtest graphviz |
There was a problem hiding this comment.
do we still need _openmp_mutex==_llvm here and .github/workflows/ci-downstream-find-package.yml?
There was a problem hiding this comment.
Yes, still needed in both, and it's not redundant with libopenblas=*=*openmp* — they pin two different things:
libopenblas=*=*openmp*picks the OpenMP-threaded OpenBLAS build variant over the pthreads one._openmp_mutex=*=*_llvmpicks which concrete OpenMP runtime (llvm'slibompvs. GNU'slibgomp) that OpenBLAS build and the-fopenmp-compiled cytnx/HPTT code both resolve to. Without it the solver could still land on_openmp_mutex=*_gnu, and the two OpenMP consumers wouldn't necessarily share one physical runtime even though both are individually "openmp-based" (see thelibgomp.so.1→libomp.sosymlink mechanism discussed in the P2 review thread above).
ci-downstream-find-package.yml already had _openmp_mutex=*=*_llvm but was missing the libopenblas=*=*openmp* pin ci-cmake_tests.yml has — added it in a0ffe55 for consistency (its own TODO(deps-single-source) comment already flags this dependency list as a near-duplicate of ci-cmake_tests.yml's).
Generated by Claude Code
There was a problem hiding this comment.
We use libgomp so we can remove _openmp_mutex==_llvm or pick gomp one.
There was a problem hiding this comment.
You're right that the pin should go — I removed _openmp_mutex=*=*_llvm from both ci-cmake_tests.yml and ci-downstream-find-package.yml in dc316cb. But I need to correct my previous reply here: it was wrong on two counts, and I verified the actual solver behavior with a micromamba create --dry-run this time rather than reasoning about it.
- "pick gomp one" isn't possible. conda-forge's OpenMP OpenBLAS build hard-requires the llvm mutex —
_openmp_mutex=*=*_gnutogether withlibopenblas=*=*openmp*fails to solve outright:libopenblas =* *openmp* is not installable because it requires _openmp_mutex =* *_llvm, which conflicts... - The explicit
_llvmpin was redundant, not load-bearing (contra what I said before). Resolving theci-cmake_tests.ymlspec with the pin removed still pulls_openmp_mutex 4.5 7_kmp_llvm+llvm-openmptransitively fromlibopenblas=*=*openmp*alone. So dropping it changes nothing about which runtime is selected.
On "we use libgomp": libgomp.so.1 is present, but under _kmp_llvm it's a symlink to libomp.so, so the physical runtime everything (GCC's -fopenmp -lgomp and OpenBLAS) resolves to is LLVM's libomp — which is exactly what makes the single-thread-pool / no-oversubscription property hold. Comments in both files now record this instead of the old (incorrect) "the pin picks the runtime" rationale.
Generated by Claude Code
ci-cmake_tests.yml already pins libopenblas=*=*openmp* alongside _openmp_mutex=*_llvm so the solver can't substitute a pthreads-threaded OpenBLAS build. ci-downstream-find-package.yml's own native-dependency install (a near-duplicate of ci-cmake_tests.yml's, per its own deps-single-source TODO comment) only had the _openmp_mutex pin, so it could resolve a different OpenBLAS threading backend than the test suite despite pinning the same OpenMP runtime. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a0ffe55388
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…scan tools/cibuildwheel_before_all_macos.sh's minos probe walks every dylib under the conda-forge prefix to compute MACOSX_DEPLOYMENT_TARGET. ccache was installed into that same prefix, so any of its own transitive dependencies -- never linked into or vendored with the wheel -- could inflate max_minos above what the actual runtime dependencies (openblas/arpack/boost/llvm-openmp) require, needlessly narrowing the wheel's supported macOS range. Moves ccache into a separate /tmp/cytnx-build-tools prefix so the scan only ever sees the wheel's real runtime dependency closure, and points pyproject.toml's PATH override at the new prefix. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
conda-forge's openmp OpenBLAS build (libopenblas=*=*openmp*) hard- requires _openmp_mutex=*_llvm: an explicit _gnu pin is unsolvable against it, and the solver already pulls _openmp_mutex 7_kmp_llvm plus llvm-openmp transitively from libopenblas=*=*openmp* alone (verified by dry-run resolving the ci-cmake_tests.yml spec with the explicit pin removed). The separate _openmp_mutex=*_llvm spec was therefore redundant in both ci-cmake_tests.yml and ci-downstream-find-package.yml. Remove it and rewrite the comments to record that libopenblas=*=*openmp* is what forces the single llvm OpenMP runtime, and that libgomp is present only as a _kmp_llvm-provided symlink to libomp (so GCC's -fopenmp -lgomp and OpenBLAS still resolve to that one runtime) rather than an independent GNU runtime. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Heads-up on an interaction with #1060 (enable ASan in This PR pins LLVM libomp makes a one-time internal allocation at its first parallel region ( Evidence (local,
Whichever of the two merges second trips this. I've pushed the openmp pin + Reported by Claude (agent session operated by @IvanaGyro). Generated by Claude Code |
|
Don't preload GNU libgomp. Using LSan suppression Note that one-off allocations are not normally regarded as leaks as such anyway. For example, the dynamic dispatch tables used by Cytnx, I am slightly surprised that these are freed at program termination -- I guess they use some global “Every allocation must be explicitly freed so LSan passes without suppressions” forbids a legitimate process-lifetime allocation strategy. The better policy is:
|
|
I didn't dig into it. The CI test didn't report LSan errors. Somehow the test workflow links with libgomp instead of libomp from conda-forge, so there is no issue there. However, the message is still good for develops to solve the problem. |
3 participants
Problem
Two independent problems traced back to where the Linux and macOS wheel builds source their native dependencies from:
tools/cibuildwheel_before_all.shinstallsboost-devel openblas-devel arpack-develfrom EPEL viadnfinside themanylinux_2_28container. EPEL'sarpack-devellinks whichever OpenBLASdnfresolves — a pthreads-threaded build with its own independent thread pool. HPTT and cytnx's own code use OpenMP (-fopenmp, seeCytnxBKNDCMakeLists.cmake), so a BLAS call made from inside an OpenMP region oversubscribes the CPU (two independent thread pools competing).tools/cibuildwheel_before_all_macos.shinstallsarpack/boost/openblas/libompfrom Homebrew and computesMACOSX_DEPLOYMENT_TARGETfrom theirminos. Homebrew's bottles are built against whatever deployment target Homebrew itself currently targets, which trends upward release over release — Cytnx's actual minimum supported macOS version was rising as a side effect of Homebrew's own build policy, not a decision made here. Homebrew's OpenBLAS bottle is also pthreads-only, same issue as Linux.Fix
Both scripts now provision a self-contained
micromambaenvironment from conda-forge instead:openblas=*=*openmp*selects the OpenMP-threaded OpenBLAS build.arpack=*=nompi*avoids vendoring an MPI runtime (conda-forge'sarpackalso hasmpich/openmpivariants).liblapacke/libboost-headersreplace the corresponding EPEL/Homebrew packages.llvm-openmpreplaces Homebrew'slibomp, and the existing dylib-minos-probing logic incibuildwheel_before_all_macos.shis kept unchanged, just repointed at the new conda-forge prefix —MACOSX_DEPLOYMENT_TARGETkeeps tracking whatever floor the actual vendored dylibs require (conda-forge'sclang_osx-64/clang_osx-arm64toolchain targets a pinned, deliberately old SDK) instead of a value guessed in advance.A second, small commit pins
ci-cmake_tests.yml's ownmamba installtolibopenblas=*=*openmp*explicitly. That workflow already pins_openmp_mutex=*=*_llvm(so the environment settles on one OpenMP runtime), butblas=*=openblasalone doesn't guarantee the solver picks the OpenMP-threaded build over the pthreads one — both are solver-compatible with_openmp_mutex=llvmpresent, since a pthreads OpenBLAS doesn't touch OpenMP at all.Threading verification (per repo convention: measure, don't assume)
Sandbox constraints: no docker registry access (egress policy) and no macOS host, so a real
cibuildwheelrun wasn't possible locally. What was verified directly, using a realmicromamba-created environment with this exact package set and realcmake/g++on the sandbox host:find_library(arpack),find_package(BLAS BLA_VENDOR OpenBLAS),find_package(LAPACK),find_package(LAPACKE), andfind_package(Boost)(forced to MODULE mode, since manylinux has no competing system Boost config) all resolve correctly against the conda-forge prefix viaCMAKE_PREFIX_PATH.libblas.so.3/liblapack.so.3/liblapacke.so.3/libopenblas.so.0are confirmed to all symlink to the same physicallibopenblasp-r0.3.33.so— one OpenBLAS copy, not two.openmp-variant OpenBLAS declaresNEEDED libgomp.so.1, but the environment's_openmp_mutex(llvm variant, conda-forge's current default for this package combination) makeslibgomp.so.1a symlink tolibomp.so(LLVM's runtime). Verifiedlibomp.sogenuinely exports the full versionedGOMP_1.0–GOMP_5.0.1symbol set GCC's-fopenmpcode needs (nm -D), so GCC-compiled cytnx code and OpenBLAS end up sharing one thread pool rather than two.arpack+ LAPACKE + Boost + OpenMP, callingLAPACKE_dgeqrffrom inside an 8-way#pragma omp parallel forregion — correct output, correct thread count, no crashes, no missing-symbol errors.ci-cmake_tests.ymlalready pins_openmp_mutex=*=*_llvmfor its own mamba-based CPU environment — i.e. this exact OpenMP-runtime combination is already proven in this repo's production CI, not a speculative first use.The macOS script itself couldn't be executed locally (no Mac available in the sandbox), so it relied on reusing the already-proven Homebrew-era probing logic verbatim, repointed at a new prefix. This PR's own CI caught three real bugs that local verification couldn't have found: the ARM64 micromamba download picking an x86_64 binary, a macOS CMake export-safety error from the deps prefix living inside the checkout, and cibuildwheel's
environmenttable not expanding${VAR:-default}shell syntax the way a real shell does. All three are fixed (see commit history) and all fourBuildWheellegs (ubuntu-24.04,ubuntu-24.04-arm,macos-14,macos-15-intel) are now green, along with every other check on the PR.Testing
pre-commit runon all changed files (end-of-file-fixer, trailing-whitespace, large-file check; no C++ touched).yaml.safe_load,tomllib.load).release_pypi.yml'sBuildWheel-*jobs,conda_build.yml'sBuildAndTest-*jobs) — all green as of the latest commit.Generated by Claude Code