Skip to content

Commit 6fd9d98

Browse files
ChrisRackauckas-ClaudeChrisRackauckasclaude
authored
Fall back from unsafe_trunc in UniformStep kernel for non-float ratio types (#101)
* Fall back from unsafe_trunc in UniformStep kernel for non-float ratio types The closed-form UniformStep kernels compute an integer index offset with `unsafe_trunc(Int, floor(f))` / `unsafe_trunc(Int, ceil(f))`. `unsafe_trunc` is only defined for hardware floats and `BigFloat`; for a `Rational` ratio type it has no method, so a uniform `Rational` vector — which `Auto` resolves to `KIND_UNIFORM_STEP` — threw a `MethodError` on every search. `f` is already clamped to the in-range interval before truncation, so replace the raw `unsafe_trunc` with a small dispatch helper: keep `unsafe_trunc` on the `Base.IEEEFloat` hot path (in-range, so the range check is redundant) and fall back to the checked `floor(Int, ·)` / `ceil(Int, ·)` for every other ordered `Real` — which those types support and which is exact for the clamped `f`. The IEEEFloat hot path is unchanged. Adds a UniformStep regression testset over uniform `Rational{Int}`, `Rational{BigInt}`, and `BigFloat` vectors, checking search results against `Base`. Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FvBXVKGujjeaCB3iLwsDeG * QA: avoid non-public Base.IEEEFloat in UniformStep truncation helper ExplicitImports' `all_qualified_accesses_are_public` check flags `Base.IEEEFloat` — it is neither exported nor declared `public` in Base. Spell out the hardware-float union `Union{Float16, Float32, Float64}` (each of which is public) via a local `const` instead. No behavior change. Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FvBXVKGujjeaCB3iLwsDeG --------- Co-authored-by: ChrisRackauckas-Claude <accounts@chrisrackauckas.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 1351a3a commit 6fd9d98

2 files changed

Lines changed: 46 additions & 2 deletions

File tree

src/kernels.jl

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -579,6 +579,20 @@ end
579579
# BinaryBracket.
580580
# ===========================================================================
581581

582+
# Truncate a guess coordinate `f` to an `Int` offset. The caller has already
583+
# clamped `f` to `[0, nm1)` (last) / `(0, nm1]` (first), so on the hardware
584+
# floats the fast `unsafe_trunc` is in-range and skips a redundant check.
585+
# Other ordered-`Real` ratio types — `Rational`, AD `Dual`, `BigFloat`, … —
586+
# do not all define `unsafe_trunc`, so fall back to the checked
587+
# `floor(Int, ·)` / `ceil(Int, ·)`, which every such type supports and which
588+
# is exact for the in-range `f`. The hardware-float union is spelled out
589+
# rather than via the non-public `Base.IEEEFloat` alias.
590+
const _HardwareFloat = Union{Float16, Float32, Float64}
591+
@inline _uniform_floor_index(f::_HardwareFloat) = unsafe_trunc(Int, floor(f))
592+
@inline _uniform_floor_index(f) = floor(Int, f)
593+
@inline _uniform_ceil_index(f::_HardwareFloat) = unsafe_trunc(Int, ceil(f))
594+
@inline _uniform_ceil_index(f) = ceil(Int, f)
595+
582596
@inline function _kernel_last_uniform_step_props(
583597
props::SearchProperties, v::AbstractVector, x, order::Base.Order.Ordering,
584598
)
@@ -605,7 +619,7 @@ end
605619
elseif f >= nm1
606620
lastindex(v)
607621
else
608-
firstindex(v) + unsafe_trunc(Int, floor(f))
622+
firstindex(v) + _uniform_floor_index(f)
609623
end
610624
# Walk to the true cell. For exactly-uniform data this takes at most
611625
# one step (float roundoff); it also keeps the result correct when
@@ -640,7 +654,7 @@ end
640654
elseif f > nm1
641655
lastindex(v) + 1
642656
else
643-
firstindex(v) + unsafe_trunc(Int, ceil(f))
657+
firstindex(v) + _uniform_ceil_index(f)
644658
end
645659
@inbounds while i > firstindex(v) && !Base.Order.lt(order, v[i - 1], x)
646660
i -= 1

test/core_tests.jl

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1445,3 +1445,33 @@ end
14451445
@test F.findfirstsortedequal(x, v) == ref
14461446
end
14471447
end
1448+
1449+
@safetestset "UniformStep kernel across non-primitive Real eltypes" begin
1450+
using FindFirstFunctions
1451+
using FindFirstFunctions: SearchProperties, Auto, searchsorted_last, searchsorted_first
1452+
1453+
# `Rational` ratio types don't define `unsafe_trunc(Int, ·)`, so a uniform
1454+
# `Rational` vector — which `Auto` resolves to the closed-form
1455+
# `KIND_UNIFORM_STEP` path — must fall back to `floor/ceil(Int, ·)` rather
1456+
# than erroring. `BigFloat` exercises the non-`IEEEFloat` AbstractFloat
1457+
# branch of the same helper.
1458+
uniform_vectors = (
1459+
Rational{Int}[i // 4 for i in 0:4:160],
1460+
Rational{BigInt}[big(i) // 4 for i in 0:4:160],
1461+
BigFloat[BigFloat(i) for i in range(0.0, 10.0; length = 40)],
1462+
)
1463+
for v in uniform_vectors
1464+
@test SearchProperties(v).is_uniform
1465+
auto = Auto(v)
1466+
@test auto isa Auto
1467+
for i in eachindex(v)
1468+
@test searchsorted_last(auto, v, v[i]) == searchsortedlast(v, v[i])
1469+
@test searchsorted_first(auto, v, v[i]) == searchsortedfirst(v, v[i])
1470+
end
1471+
# Between-knot and out-of-range queries.
1472+
for q in (v[1] - one(eltype(v)), (v[3] + v[4]) / 2, v[end] + one(eltype(v)))
1473+
@test searchsorted_last(auto, v, q) == searchsortedlast(v, q)
1474+
@test searchsorted_first(auto, v, q) == searchsortedfirst(v, q)
1475+
end
1476+
end
1477+
end

0 commit comments

Comments
 (0)