Retire legacy-Accelerate accommodations: macOS runs the full library - #155
Draft
mmelnich wants to merge 1 commit into
Draft
Retire legacy-Accelerate accommodations: macOS runs the full library#155mmelnich wants to merge 1 commit into
mmelnich wants to merge 1 commit into
Conversation
Apple's legacy Accelerate LAPACK (3.2.1, 2009) breaks gesdd on Apple Silicon and lacks orhr_col/geqrt & co., which forced 18 __APPLE__ guards into the codebase: BQRRP threw 'not supported' on every Mac, HQRRP's CholQR path was stubbed, CQRRPT could not delegate to BQRRP, test_bqrrp and test_orhr_col were compiled out, and ten benchmarks were stub mains. Apple's new interface (ACCELERATE_NEW_LAPACK, macOS >= 13.3, LAPACK 3.12 on current SDKs) fixes all of it -- verified end to end: 311/311 tests pass on macOS including the gesdd canary, all of TestBQRRP, and test_orhr_col. Remove every guard; switch core-macos to the new interface (-Dblas=accelerate + a hard defines.h check so a silent fallback to the legacy interface fails the job); document the macOS requirement in INSTALL.md. Requires BLAS++/LAPACK++ with new-Accelerate support, so this stays unmerged until icl-utk-edu merges the two PRs from BallisticLA:new-accelerate -- core-macos here is deliberately red until then and flips green by itself when upstream lands.
mmelnich
added a commit
that referenced
this pull request
Aug 12, 2026
…ark's BLAS++/LAPACK++ review) (#157) > [!WARNING] > **This is a temporary change and is meant to be reverted.** > It is one clean `git revert` of a single commit. ### Revert when these land The quarantine exists only until Apple's new Accelerate interface is in use. Both are open and awaiting review: | PR | Status | |---|---| | [icl-utk-edu/blaspp#134](icl-utk-edu/blaspp#134) — New Apple Accelerate support (#74, rebased and completed) | open | | [icl-utk-edu/lapackpp#88](icl-utk-edu/lapackpp#88) — Support Apple's new Accelerate interface (closes #43) | open | **Order matters:** lapackpp#88 depends on the `defines.h` from blaspp#134, so #134 merges first. blaspp#134 is a rebased and completed continuation of the stalled [blaspp#74](icl-utk-edu/blaspp#74). Once both are merged, [RandLAPACK#155](#155) (retire legacy-Accelerate accommodations) becomes mergeable and `TestQB.Polynomial_Decay_general1` should pass on its own — at which point the warning added here fires and tells you to delete this quarantine. ## Why `TestQB.Polynomial_Decay_general1` fails on Apple Silicon because Apple's old Accelerate LAPACK has a broken divide-and-conquer `gesdd`. It has been left failing deliberately, as a canary for the planned migration. The side effect is that **`core-macos` has been permanently red** — `build` and `build-asan`, on `main` and on every branch. A lane that is always red carries no information: you cannot distinguish a new macOS regression from the known one, so the job gets ignored and the canary signals to nobody. ## What this does Quarantines the test without discarding the signal: - The main `ctest` run excludes it, so the job reflects real macOS health. - The test is then **still run**, separately, and cannot fail the job. - Every macOS run prints a warning that the suppression is active and names the condition for removing it. - If the test ever **passes**, the job prints a louder warning saying to delete the quarantine. That last point matters: the migration signal is preserved and actually improved. Previously it was "a red job turns green", which nobody was watching because the job was always red. Now it is an explicit annotation telling you what to delete. ## Scope Deliberately surgical — `core-macos.yaml` (both jobs) and the corresponding `docs/CI.md` entry, nothing else, so the revert stays clean. The dead `TestABRIK.ABRIK_catch_instability` exclusion in these same lines is left alone for that reason.
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
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.
⛔ Do not merge yet
This PR must stay open until icl-utk-edu merges the new-Accelerate support in BLAS++ and LAPACK++ (the PRs from
BallisticLA:new-accelerateon both repos). It removes every legacy-Accelerate accommodation from RandLAPACK, so a macOS build only works against a BLAS++/LAPACK++ that can select Apple's new interface. The core-macos job here deliberately builds from the icl-utk-edu masters: it stays red until upstream merges, and flips green by itself when they do — that flip is the merge signal.What this PR does
Makes macOS a full-library, first-class platform by removing all 18
__APPLE__guards, and switches macOS CI to Apple's new Accelerate interface.Background. Apple ships two LAPACKs in Accelerate. The legacy default is 3.2.1 (2009):
gesddreturns wrong results on Apple Silicon (the reason core-macos has carried a deliberately red canary test), and routines RandLAPACK needs (orhr_col,geqrt, ...) don't exist — the reason BQRRP threw "not supported" on every Mac and ten benchmarks were stubbed out. The new interface (macOS ≥ 13.3,ACCELERATE_NEW_LAPACK, LAPACK 3.12 on current SDKs) fixes both, verified on this repo's CI: run 31065130007 — 311/311 tests pass on macOS, including the gesdd canary, all of TestBQRRP (first macOS execution ever), andtest_orhr_col.Changes:
test_bqrrp(whole file), the guarded sections oftest_cqrrpt/test_hqrrp/test_bqrrp_gpu, andtest_orhr_colnow compile everywhere — macOS gains 16 tests.-Dblas=accelerateand hard-fails if the new interface wasn't selected (no silent legacy fallback). The canary test is included and expected green — the canary era ends with this PR.install.shon macOS uses OpenBLAS and is unaffected).Merge order: icl-utk-edu/blaspp PR → icl-utk-edu/lapackpp PR → core-macos here turns green → merge this.