Repository navigation
test(search): compare cubic values across search policies with a ULP budget - #212
Merged
Merged
Conversation
…budget The "Integrated test" pinned the cubic results of six search policies with `==` on an unseeded `sort(rand(10))`. The policies share the inlined value kernel, but each specialization is optimized as its own unit, and on Julia 1.13 under `--check-bounds=yes` LLVM packs the linear blend of the LinearBinarySearch specialization into a 2-wide vector with a horizontal add, which cannot fuse. The default and LinearSearch paths keep both FMAs, so the values drift by 1-2 ULP on ~17 % of queries and the pin failed on ~85 % of seeds (macOS 1.x shard 2/2) since the t-form cubic kernel landed. Cross-path value pins use the `isclose(…; nulps = PATH_ULPS)` budget from the `Basic` snippet, as the other search-policy tests already do.
There was a problem hiding this comment.
🟢 Approval recommended
The focused test-only change consistently applies the repository’s existing cross-path tolerance policy.
0 open findings
What changed in this PR
Updates search-policy parity tests to tolerate expected floating-point contraction differences.
Changes:
- Imports shared
Basictest helpers. - Compares cubic outputs using the established 8-ULP path budget.
| File | Description |
|---|---|
test/test_search.jl |
Replaces exact cross-policy value comparisons with isclose. |
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
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
Problem. Since #211 the
Julia 1.x macos-latest [2/2]shard fails on most runs (master969b26cb0, and #209 after its rebase):test/test_search.jl:671-674reports cubic values 1-2 ULP apart betweenLinearBinarySearch{0,2,8}and the defaultBinarySearch.Cause. The "Integrated test" pinned six search policies with
==on an unseededsort(rand(10)). All policies share the inlined cubic value kernel, but each specialization is optimized as its own unit: on Julia 1.13 under--check-bounds=yes(whatPkg.testpasses) LLVM's SLP vectorizer packs the linear blend of theLinearBinarySearchspecialization into a<2 x double>multiply/subtract plus a horizontal add, which cannot fuse, while the default andLinearSearchpaths keep both FMAs. The old dL³/dR³ kernel happened to compile identically on every path, so the pin never flaked; the t-form kernel from #211 exposes it on ~17 % of queries, i.e. ~85 % of seeds for a 10-query vector. Vector-grid search code performs no float arithmetic, so this is optimizer context only;muladdis contraction-optional by definition.Fix. The five cross-path value pins use the
isclose(…; nulps = PATH_ULPS)budget from theBasicsnippet, the policy the other search-policy tests already follow. Search/index pins in the file stay exact.--check-bounds=yes, 200 seedsChanges
test/test_search.jl:@testitem "Search Module"gainssetup = [Basic]; the five==pins becomeisclose(…; nulps = PATH_ULPS).Not covered
src/is untouched. Kernel-only accuracy of the fix(cubic): exact node values from the cubic value kernel #211 kernel against a BigFloat reference of the same inputs (100k queries,y = x³on 101 nodes) is better than the old kernel on both paths: mean abs error 0.0622 eps (default) / 0.0693 eps (LinearBinarySearch) vs 0.0716 eps before.test_nd_autosearch_peraxis.jl:195/386keep==across linear search paths; they have stayed green and are left as is.Tests
cc-julia-test-runner . test_search.jlgreen locally (aarch64, Julia 1.13.0);runic --checkclean.--check-bounds=yes:==fails 171/200 seeds,isclose(PATH_ULPS)0/200, max drift 2 ULP.