diff --git a/.github/workflows/rstsr-openblas-test.yml b/.github/workflows/rstsr-openblas-test.yml index 1a6fbb53..b57d50de 100644 --- a/.github/workflows/rstsr-openblas-test.yml +++ b/.github/workflows/rstsr-openblas-test.yml @@ -37,3 +37,43 @@ jobs: python func_validation_f64.py - name: test run: RSTSR_DEV=1 cargo test -p rstsr-openblas --release --features="openmp linalg" + + # Default features of `rstsr-openblas` are `linalg` + `openmp`; this job runs + # them against a pthread-built OpenBLAS, which takes the `OPENBLAS_THREAD` + # runtime path (the OpenMP runtime is still linked, but unused by OpenBLAS). + unittests-pthread: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: conda-incubator/setup-miniconda@v2 + with: + miniconda-version: "latest" + - name: install dependencies + run: sudo apt-get install -y libopenblas-pthread-dev + - name: conda environment setup + run: | + conda init; source $HOME/.bashrc; conda activate + conda install -y numpy scipy + - name: manifest initialization + run: | + conda init; source $HOME/.bashrc; conda activate + cd rstsr-test-manifest/resources + python gen_rand_vec.py + - name: validate manifest (driver_validation_f64) + run: | + conda init; source $HOME/.bashrc; conda activate + cd crates-device/rstsr-openblas/tests/driver_impl + python driver_validation_f64.py + - name: validate manifest (func_validation_f64) + run: | + conda init; source $HOME/.bashrc; conda activate + cd crates-device/rstsr-openblas/tests/linalg_func + python func_validation_f64.py + - name: test + run: RSTSR_DEV=1 cargo test -p rstsr-openblas --release -- --test-threads=1 + env: + # Ubuntu noble's libopenblas0-pthread (0.3.26) overflows libtest's default + # 2 MiB test-thread stack in driver_impl LAPACK tests (pthread builds run + # parallel LAPACK work on the calling thread's stack). Local 0.3.34 pthread + # builds pass even with 512 KiB, so this is specific to the distro build. + RUST_MIN_STACK: 16777216 diff --git a/crates-device/rstsr-openblas/Cargo.toml b/crates-device/rstsr-openblas/Cargo.toml index e62e22c8..ea7fa81e 100644 --- a/crates-device/rstsr-openblas/Cargo.toml +++ b/crates-device/rstsr-openblas/Cargo.toml @@ -28,7 +28,7 @@ rstsr = { path = "../../rstsr", default-features = false, features = ["openblas" rstsr-test-manifest = { workspace = true } [features] -default = ["linalg"] +default = ["linalg", "openmp"] dynamic_loading = ["rstsr-openblas-ffi/dynamic_loading"] faer = ["rstsr-core/faer"] ilp64 = ["rstsr-openblas-ffi/ilp64", "rstsr-blas-traits/ilp64"] diff --git a/crates-device/rstsr-openblas/readme.md b/crates-device/rstsr-openblas/readme.md index 4556a2c6..50d0d680 100644 --- a/crates-device/rstsr-openblas/readme.md +++ b/crates-device/rstsr-openblas/readme.md @@ -33,8 +33,29 @@ assert!((c_mean - 213.2503660477036) < 1e-6); - We do not provide automatic linkage: - Please add `-l openblas` in `RUSTFLAGS`, or `cargo:rustc-link-lib=openblas` in build.rs, or something similar, to your project. We do not use external FFI crates `blas` or `blas-sys`, and do not automatically search OpenBLAS library for linking. - - If feature `openmp` activated, please add `-l gomp` or `-l omp` in `RUSTFLAGS`, or `cargo:rustc-link-lib=gomp` or `cargo:rustc-link-lib=omp` in build.rs, or something similar, to your project. + - The `openmp` feature is a default feature (since 0.8.0), so an OpenMP runtime must also be linked: `gomp` on Linux (GNU toolchain), `omp` on macOS (LLVM), `vcomp` on Windows (MSVC). + Please add `-l gomp` or `-l omp` in `RUSTFLAGS`, or `cargo:rustc-link-lib=gomp` or `cargo:rustc-link-lib=omp` in build.rs, or something similar, to your project. We do not use external FFI crate `openmp-sys`, and do not automatically search for OpenMP library for linking. -- If your OpenBLAS is compiled with OpenMP, please add `openmp` feature to either this crate or `rstsr-openblas-ffi`. - - In our testing, OpenBLAS with OpenMP is probably more efficient than pthreads. However, we currently decided not make `openmp` as default feature. \ No newline at end of file +- Why `openmp` by default, and what it costs you: + - In our testing, OpenBLAS compiled with OpenMP is generally more efficient than the pthread build, and OpenBLAS binaries built either way are common in the wild; with `openmp` enabled the crate handles both at runtime. + - The feature is **compatible with a pthread-built OpenBLAS**: the runtime reports `OPENBLAS_THREAD` and the pthread API (`openblas_set_num_threads`) is used; the linked OpenMP runtime simply stays unused. + - The only cost is the link requirement above. If you do not want it, disable default features and re-enable what you need: + + ```toml + rstsr-openblas = { version = "0.8", default-features = false, features = ["linalg"] } + ``` + + Note that cargo feature unification is graph-global: if any other crate in your build enables `rstsr-openblas` default features, `openmp` is on for the whole build, and your opt-out alone will not remove the link requirement. + - With `openmp` (and `dynamic_loading`) disabled, an OpenMP-built OpenBLAS makes the threading API panic with migration guidance. If you cannot link an OpenMP runtime at all, either use a pthread-built OpenBLAS, or enable `dynamic_loading` and resolve everything at runtime. + +- Compatibility matrix (verified against OpenBLAS 0.3.34 built both ways, Linux/GNU toolchain): + +| OpenBLAS build | `openmp` feature | OpenMP runtime linked | result | +| --- | --- | --- | --- | +| OpenMP | on (default) | yes | works; threading via `omp_*` | +| OpenMP | on | no | link error (`omp_*` undefined; OpenBLAS's own dependency on libgomp does not resolve them for you) | +| OpenMP | off | no | links; threading API panics with guidance | +| pthread | on (default) | yes | works; threading via `openblas_*`, OpenMP runtime unused | +| pthread | on | no | link error (`omp_*` undefined) | +| pthread | off | no | works; no OpenMP runtime needed | \ No newline at end of file diff --git a/crates-device/rstsr-openblas/src/threading.rs b/crates-device/rstsr-openblas/src/threading.rs index f710deb2..f6a606ac 100644 --- a/crates-device/rstsr-openblas/src/threading.rs +++ b/crates-device/rstsr-openblas/src/threading.rs @@ -31,11 +31,11 @@ pub fn get_parallel() -> OpenBLASParallel { OpenBLASParallel::OpenMP } else { panic!(concat!( - "OpenMP is not enabled in `rstsr-openblas-ffi`, but detected using shared library `libopenblas` compiled with OpenMP.\n", + "OpenMP-built `libopenblas` detected, but this crate was built without features `openmp` or `dynamic_loading`.\n", "Please either\n", + "- enable feature `openmp` of `rstsr-openblas` (a default feature of this crate; the integration crate `rstsr` also enables it by default whenever its `openblas` feature is on — with non-default `rstsr` features, add it via a direct `rstsr-openblas` dependency), and link the OpenMP runtime (e.g. `gomp` on Linux, `omp` on macOS) in build.rs or `RUSTFLAGS`;\n", "- enable feature `dynamic_loading` when building `rstsr-openblas` and rebuild this crate, and everything will be determined at runtime;\n", - "- enable feature `openmp` when building `rstsr-openblas` and rebuild this crate, with OpenMP library linked;\n", - "- run with libopenblas compiled with pthread (rebuild `rstsr-openblas-ffi` is not required in this case).", + "- run with libopenblas compiled with pthread, and no OpenMP runtime is needed.", )) } }, @@ -80,16 +80,16 @@ impl OpenBLASConfig { Some(p) => p, None => { let p = unsafe { rstsr_openblas_ffi::cblas::openblas_get_parallel() } as u32; - if cfg!(any(feature = "openmp", feature = "dynamic_loading")) { + if cfg!(any(feature = "openmp", feature = "dynamic_loading")) || p != OPENBLAS_OPENMP { self.parallel = Some(p); p } else { panic!(concat!( - "OpenMP is not enabled in `rstsr-openblas-ffi`, but detected using shared library `libopenblas` compiled with OpenMP.\n", + "OpenMP-built `libopenblas` detected, but this crate was built without features `openmp` or `dynamic_loading`.\n", "Please either\n", + "- enable feature `openmp` of `rstsr-openblas` (a default feature of this crate; the integration crate `rstsr` also enables it by default whenever its `openblas` feature is on — with non-default `rstsr` features, add it via a direct `rstsr-openblas` dependency), and link the OpenMP runtime (e.g. `gomp` on Linux, `omp` on macOS) in build.rs or `RUSTFLAGS`;\n", "- enable feature `dynamic_loading` when building `rstsr-openblas` and rebuild this crate, and everything will be determined at runtime;\n", - "- enable feature `openmp` when building `rstsr-openblas` and rebuild this crate, with OpenMP library linked;\n", - "- run with libopenblas compiled with pthread (rebuild `rstsr-openblas-ffi` is not required in this case).", + "- run with libopenblas compiled with pthread, and no OpenMP runtime is needed.", )) } }, diff --git a/rstsr/Cargo.toml b/rstsr/Cargo.toml index 75d66c81..a46ac2e7 100644 --- a/rstsr/Cargo.toml +++ b/rstsr/Cargo.toml @@ -31,7 +31,7 @@ rstsr-tblis = { workspace = true, optional = true } [dev-dependencies] [features] -default = ["std", "backtrace", "rstsr-core/default", "faer", "faer_as_default"] +default = ["std", "backtrace", "rstsr-core/default", "faer", "faer_as_default", "rstsr-openblas?/openmp"] # rstsr-core features std = ["rstsr-core/std"]