INSTALL_SCRIPT.md: document the new options, and add tested configurations - #164
Merged
Merged
Conversation
…tions The guide described a script that no longer exists in several respects. Four changes. The documented flag list predated backend selection entirely. It now covers --blas, --blas-int, --blas-libraries, --prefix, --no-extras, --no-benchmarks and --no-progress, and states that extras and benchmarks are built by default with opt-outs -- deliberately unlike RandBLAS's examples, which are opt-in because they pull in dependencies RandBLAS itself does not need. The directory-structure diagram said the script would MOVE your clone into lib/. It no longer does -- that broke git worktrees -- and lib/RandLAPACK is a symlink. The diagram also now shows the install/ tree, whose dependency directories carry the backend and GPU configuration in their names. "What the Script Does" described cloning dependencies from their default branches and moving the RandLAPACK directory. It now describes the pinned fetches, the provenance stamps that make reuse configuration-aware, and the verification step -- including why that step runs a program rather than only linking one. New section 6 covers what the earlier guide had nowhere: a tested-configuration table drawn from the CI lanes, the per-backend integer width with the reason OpenBLAS is the awkward case, whether LP64 actually limits RandLAPACK (rarely, and loudly -- with the genuinely silent header/library mismatch called out separately), and why macOS defaults to Homebrew OpenBLAS rather than Accelerate. Section 6 is placed before the GPU-benchmark appendix so the numbered sections stay contiguous.
Base automatically changed from
randlapack-install-ps1
to
randlapack-install-sh
August 13, 2026 20:30
Contributor
Author
|
For the record: this shows as merged rather than closed because #162's branch was fast-forwarded to contain it, so its commits are genuinely in that branch — it merged into its stacked base, not into Max asked for #162, #163 and #164 to land as one PR, matching how the RandBLAS installer set was consolidated. All of this work is now reviewable in #162. |
This was referenced Aug 13, 2026
mmelnich
added a commit
that referenced
this pull request
Aug 13, 2026
… backend selection, and docs (#162) ## Problem RandLAPACK's two installers had drifted apart. `install/install.ps1` was rebuilt in #156 and pins every dependency, checksums downloads, records provenance and link-and-run validates the BLAS before building anything. `install/install.sh` did none of that, and the documentation described a script that no longer existed in several respects. Eleven defects, all verified against `main` before touching anything: | Where | Defect | |---|---| | `install.sh:336,339,342` | three `git clone` with **no ref** — blaspp, lapackpp, random123 all tracked upstream default branches | | — | **no BLAS validation at all** | | — | `/opt/homebrew` hardcoded, **8 places** — hard-fails on Intel macOS and custom `HOMEBREW_PREFIX` | | `install.sh:331` | `mv "$REPO_DIR"` relocated the user's own clone, breaking git worktrees | | `install.sh:211` | `: > "$LOG"` truncated the log, destroying the previous run's output | | `install.sh:247` | libdir search omitted `lib/aarch64-linux-gnu` | | — | no `--prefix`; extras and benchmarks unconditional with no way to skip | | `install.ps1` | **zero** references to `RANDNLA_PROJECT_DIR`, no `--modify-rc` equivalent | | `INSTALL_SCRIPT.md` | documented the pre-backend flag list, and still told people the script would move their clone | ## The two things most worth reviewing ### Integer width is read back, not assumed BLAS++ probes `int32` before `int64`, and `blas_int` only filters which *library names* to try. For MKL that is a real choice — `mkl_intel_lp64` and `mkl_intel_ilp64` are different libraries. For OpenBLAS there is only `-lopenblas`, so **a successful `blas_int=int64` configure proves nothing**: an LP64 build passes the `int32` probe and is accepted. The resolved width therefore comes from BLAS++'s generated `blas/defines.h` after the build. Trusting the request would stamp LP64 installs as ILP64 and have every later run reuse them believing otherwise. ### Verification runs the program, and that is the point The conftest compiles, links and *runs* against the finished install — through BLAS++ and LAPACK++ rather than raw `dgemm_`, since that is how RandLAPACK reaches them — checking a `gemm` result and a `gesdd` factorization numerically. `gesdd` is chosen because it is the routine Apple's legacy Accelerate computes incorrectly. Running it rather than only linking matters for a reason specific to integer width. BLAS++ and LAPACK++ *do* guard the `int64_t` downcast — `to_blas_int` (`blaspp/src/blas_internal.hh:16-22`) throws rather than truncates — but that guard keys off `sizeof(blas_int)` **as declared by the header**. If the headers say 64-bit while the library actually loaded is LP64, the guard compiles out and 64-bit values reach routines reading 32 bits. That surfaces not as wrong numbers but as nonsense control values: a misread workspace query becomes an absurd `lwork`, and the run dies in allocation or never finishes. Nothing catches it by inspection. ## A cross-project collision, found while testing Dependency install directories now carry backend and GPU in their names (`blaspp-mkl-cpu-install`). That is not cosmetic. RandBLAS's installer uses the same `RandNLA-project` layout and named its BLAS++ install **identically** (`blaspp-<backend>-install`), with a different stamp filename. With a shared `RANDNLA_PROJECT_DIR`, this script would rebuild over RandBLAS's BLAS++ — their stamp never matches ours — and **RandBLAS's next run would reuse an artifact we had replaced underneath it, while its own stamp still described the original.** Distinct names make it impossible; sharing is instead explicit via `BLASPP_INSTALL_DIR`, verified by the conftest. ## Everything else - **macOS keeps Homebrew OpenBLAS as the default.** Apple's legacy Accelerate has a broken divide-and-conquer `gesdd`, and RandLAPACK calls `gesdd` in `rl_rsvd.hh:146`, `rl_abrik.hh:692`, `rl_revd2.hh:208`, `rl_preconditioners.hh:355` and `rl_util.hh:413`. That is what #157's quarantine is about. `--blas=accelerate` warns, citing #159. The hardcoded `/opt/homebrew` is fixed regardless. - **Extras and benchmarks stay built by default** (`--no-extras` / `--no-benchmarks`), unlike RandBLAS's examples — they need nothing this script has not already built. - **The clone is never moved**; a symlink gives the same layout and the path CI invokes still resolves. - **RandBLAS is untouched** — still a pinned submodule, still authoritative. - **`install.ps1`** now honours `RANDNLA_PROJECT_DIR` with the same precedence as `install.sh`, gains `-ModifyEnvironment` (User-scope, the only thing that survives a new shell) and `-Prefix`, and its path-length warning now checks the resolved path rather than only an explicitly passed one. - **Progress rendering** in three tiers, determinate from the build tool's own output, with tier 0 byte-identical to the plain step list — plus a CI assertion that redirected output carries no escape sequence. - **`INSTALL_SCRIPT.md`** rewritten: current flags, corrected layout diagram, and a new section 6 with a tested-configuration table drawn from the CI lanes, the per-backend integer width, whether LP64 actually limits RandLAPACK, and why macOS is on OpenBLAS. ## Verification Local Linux (gcc 15.2, oneAPI MKL, NVIDIA GPU present): | Scenario | Result | |---|---| | Fresh MKL build, `--no-gpu` | 9/9 steps, ILP64 confirmed from `blas/defines.h` | | Re-run in place | all six dependency steps reused, 1s rebuild | | Full default run (extras + benchmarks) | 13/13 steps | | `--no-gpu` → `--gpu` | BLAS++ provenance invalidated, rebuilt rather than reused | | Dependency discovery | all three reused, 3/3 steps | | Run from a git worktree | clone left in place, worktree still functional | | Output redirected | no ANSI escapes, no carriage returns | | **Full test suite** | **313/313 pass** | | RandBLAS then RandLAPACK, shared `RANDNLA_PROJECT_DIR` | both dependency sets intact | | RandLAPACK reusing RandBLAS's BLAS++ | reused, only LAPACK++ built, 5/5 steps | Windows 11, Windows PowerShell 5.1, VS 2022 Build Tools: all three `-ProjectDir` precedence cases resolve as intended, and the User-scope environment write round-trips. ## RandBLAS submodule bump Also bumps the vendored RandBLAS from `04f2018` to `952251c` (RandBLAS main), and `RandLAPACK_RandBLAS_PIN` with it. The three commits in that range are **#185** (vcpkg manifest fix), **#186** (CMake hygiene) and **#187** (installer scripts). None of them touch `RandBLAS/` headers — the library code is byte-identical, and everything in the range is build machinery. Two pieces of it matter here: - RandBLAS's CMake floor moved 3.12 → 3.21. RandLAPACK already declares 3.21, so nothing to change; the submodule now simply enforces what RandLAPACK already required. - RandBLAS exports `randblas_stage_runtime_dlls()` from its installed package and prints a configuration summary. As a subproject under RandLAPACK, which configures it with `BUILD_TESTS=OFF`, that summary reports tests as deliberately skipped rather than warning. `CMakeLists.txt` enforces that the pin variable and the submodule move together, and it means it: the cross-check compares the variable against `git ls-tree HEAD RandBLAS`, so a staged-but-uncommitted bump fails configure until both land in one commit. Worth knowing if you ever bump this iteratively — it is not satisfiable before committing. Verified: clean build against the bumped submodule, and the full suite re-run. ## Notes for reviewers - **Consolidated from #162 + #163 + #164** at Max's request, matching how the RandBLAS installer set was combined. Those two show as *merged* rather than closed because this branch was fast-forwarded to contain them — they merged into their stacked base, not into `main`. The tree is byte-identical to the state tested and green as three separate PRs. - Stacked on #161 (CI fixes), which stays separate — it is independent and useful on its own. - **`-ModifyEnvironment` is not verified end to end.** It runs at the end of a successful install, and I could not reach it: the clone sits on a `\\wsl.localhost` path and Windows `git` refuses it with `dubious ownership`. Precedence and the User-scope write are each verified directly, but not together in one completed run. - Two upstream gaps are filed rather than worked around silently: BLAS++ never looks for an ILP64 OpenBLAS (#166), and implements only Apple's legacy Accelerate interface (#165).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
INSTALL_SCRIPT.mddescribed a script that no longer exists in several respects, and omitted the things people most need to know.--blas,--blas-int,--prefix, or the extras/benchmark opt-outs.lib/. It no longer does (that broke git worktrees), andlib/RandLAPACKis a symlink.What this PR does
Documentation only; no code changes.
examples/, which are opt-in because they pull in dependencies RandBLAS itself does not need.install/tree, whose dependency directories carry backend and GPU configuration in their names.int32beforeint64andblas_intonly filters library names, so with one candidate name an LP64 OpenBLAS passes the probe and is accepted — a successfulblas_int=int64configure proves nothing. Includes the--blas=customrecipe for an ILP64 OpenBLAS, which exists but which BLAS++ never looks for.to_blas_int/to_lapack_intthrow rather than truncate, and the guard is on individual dimensions rather than element counts, so a 100,000 × 100,000 matrix is fine. The genuinely silent case — headers built for one width, library loaded with the other — is called out separately, since that is the one the verification step exists to catch.gesdd, and RandLAPACK callsgesddin five library files, so most SVD-based drivers would be affected. Cross-referenced to Revert the macOS Accelerate quarantine (PR #157) once the upstream Accelerate PRs are approved #159.Notes for reviewers
docs/INSTALL_SCRIPT.mdand also edits the rootinstall.shwrapper. I have deliberately updated the copy that exists onmainso this PR stands alone; Docs overhaul: README, ARCHITECTURE, CONTRIBUTING, install docs #153 will need to absorb it on rebase. Flagging rather than letting the merge discover it. Retire legacy-Accelerate accommodations: macOS runs the full library #155 also edits rootINSTALL.md, so three branches currently touch install docs.