Skip to content

Fall back from unsafe_trunc in UniformStep kernel for non-float ratio types#101

Merged
ChrisRackauckas merged 2 commits into
SciML:mainfrom
ChrisRackauckas-Claude:robust-uniformstep-int-truncation
Jul 13, 2026
Merged

Fall back from unsafe_trunc in UniformStep kernel for non-float ratio types#101
ChrisRackauckas merged 2 commits into
SciML:mainfrom
ChrisRackauckas-Claude:robust-uniformstep-int-truncation

Conversation

@ChrisRackauckas-Claude

Copy link
Copy Markdown
Contributor

Please ignore until reviewed by @ChrisRackauckas. Draft — opened by an agent.

Problem

A uniform Rational vector crashes on every search:

v = Rational{Int}[i//4 for i in 0:4:160]
searchsorted_last(Auto(v), v, 5//1)
# ERROR: MethodError: no method matching unsafe_trunc(::Type{Int64}, ::Rational{Int64})

Auto resolves a uniform vector to KIND_UNIFORM_STEP, and the closed-form kernels compute the 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. This is a pre-existing bug on main, independent of AD — found while testing SearchProperties/search robustness across element types (Float16/32/64, BigFloat, Rational, BigInt, Dual).

Fix

f is already clamped to the in-range interval before truncation, so the raw unsafe_trunc is replaced by a small dispatch helper:

@inline _uniform_floor_index(f::Base.IEEEFloat) = unsafe_trunc(Int, floor(f))
@inline _uniform_floor_index(f) = floor(Int, f)
@inline _uniform_ceil_index(f::Base.IEEEFloat) = unsafe_trunc(Int, ceil(f))
@inline _uniform_ceil_index(f) = ceil(Int, f)
  • Base.IEEEFloat hot path keeps unsafe_trunc (in-range, so its range check is redundant) — unchanged.
  • Every other ordered Real (Rational, BigFloat, AD Dual, …) falls back to the checked floor(Int, ·)/ceil(Int, ·), which those types support and which is exact for the clamped f.

Tests

Adds a UniformStep regression testset over uniform Rational{Int}, Rational{BigInt}, and BigFloat vectors, checking search results (exact-knot, between-knot, out-of-range) against Base.

  • Full GROUP=Core suite: 172,183 pass locally.
  • Runic clean.

Independence

Independent of the AD probe PR (#100) — touches src/kernels.jl and a disjoint region of the test file, no shared hunks, no ForwardDiff dependency. Either can merge first.

ChrisRackauckas and others added 2 commits July 13, 2026 10:17
… 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
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
@ChrisRackauckas ChrisRackauckas marked this pull request as ready for review July 13, 2026 19:57
@ChrisRackauckas ChrisRackauckas merged commit 6fd9d98 into SciML:main Jul 13, 2026
12 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants