Repository navigation
Upgrade vendored Eigen to 5.0.1 - #135
Merged
Merged
Conversation
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.
Summary
Upgrade bundled Eigen from 3.4.0 to the official 5.0.1 release for upstream fixes and maintenance updates.
rabitqlib/third/includes as system headers only during clang-tidy analysis. Header filtering alone still surfaced Eigen AVX-512 constructor diagnostics through first-party call paths. Keep first-party checks and production flags unchanged.The previous headers matched official 3.4.0 after newline normalization. All 408 imported 5.0.1 files were verified against the official archive. Existing per-ISA namespace isolation, MSVC EIGEN_DONT_VECTORIZE, public APIs, and saved-index formats are unchanged.
Testing
Windows x64, Intel Core i5-14500HX, MSVC 19.51.36257.0 / Visual Studio 2026, Release, existing Python 3.13 environment. All library/package builds used RABITQ_ENABLE_NATIVE_OPTIMIZATION=OFF. Runtime capabilities: AVX2/FMA available; AVX-512 and NEON unavailable.
Local matrix mini benchmarks used fixed Gaussian inputs (seed 42), dimensions 128/768, one thread, warmups, and seven alternating old/new rounds. Existing MSVC Eigen vectorization settings were preserved. Median GEMM times for 256 x d times d x 128 were 657.5 -> 659.7 microseconds and 3946.8 -> 3909.2 microseconds. Other principal matrix kernels were similarly close; the largest observed changes were 128-dimensional row normalization (-6.6%) and QR (+2.4%). All old/new output arrays matched exactly. These are exploratory local microbenchmarks, not an end-to-end speedup claim.
Additional validation:
UninitializedObjectdiagnostic locally with clang-tidy 22 onsrc/simd/matrix_avx512.cpp; the system-header prefix resolves it. A separate cross-file fixture verifies that the identical first-party defect still fails while the vendored defect is excluded.3375373: both shards and the aggregate gate are green (CI run).Cross-machine performance measurements are still planned; no cross-platform performance claim is made.
Known diagnostics: MSVC C4819 source-code-page warnings occur in upstream Eigen comments (also present in the 3.4.0 baseline). The staged whitespace check reports one trailing space in upstream COPYING.MPL2:38; that license is preserved verbatim, and the remaining changes pass the whitespace check.