Skip to content

Use round(::Dual, ::RoundingMode) for rounding functions on 1.11+ - #829

Merged
devmotion merged 2 commits into
JuliaDiff:masterfrom
JamesWrigley:rounding
Aug 6, 2026
Merged

Use round(::Dual, ::RoundingMode) for rounding functions on 1.11+#829
devmotion merged 2 commits into
JuliaDiff:masterfrom
JamesWrigley:rounding

Conversation

@JamesWrigley

Copy link
Copy Markdown
Contributor

The other functions like floor/trunc are already implemented in terms of that function in 1.11: https://docs.julialang.org/en/v1/manual/interfaces/#man-rounding-interface

Fixes these invalidations seen when loading CurveFit.jl on 1.13:

 inserting floor(::Type{R}, d::ForwardDiff.Dual) where R<:Real @ ForwardDiff ~/.julia/packages/ForwardDiff/IleGP/src/dual.jl:328 invalidated:                                                                                                  
   backedges: 1: superseding floor(::Type{T}, x) where T @ Base rounding.jl:484 with MethodInstance for floor(::Type{Int64}, ::Any) (658 children)                                                                                             

@codecov

codecov Bot commented Aug 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.68%. Comparing base (f1bb430) to head (af6d611).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #829      +/-   ##
==========================================
- Coverage   90.71%   90.68%   -0.03%     
==========================================
  Files          11       11              
  Lines        1055     1052       -3     
==========================================
- Hits          957      954       -3     
  Misses         98       98              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@devmotion

Copy link
Copy Markdown
Member

Thanks, this is a nice cleanup — getting rid of these invalidations is definitely worth it.

I think the version bound can be narrower than 1.11 though. Base has defined

trunc(x::Real; kwargs...) = round(x, RoundToZero; kwargs...)
floor(x::Real; kwargs...) = round(x, RoundDown; kwargs...)
ceil(x::Real; kwargs...)  = round(x, RoundUp; kwargs...)

for much longer than that (base/floatfuncs.jl:153-155 on 1.10), and since round(x::Real, r::RoundingMode=RoundNearest; ...) (base/floatfuncs.jl:128) has a default for r, the one-arg round(d) re-dispatches to round(d, RoundNearest) as well. The 1.10 docstring already recommends this:

To extend round to new numeric types, it is typically sufficient to define Base.round(x::NewType, r::RoundingMode).

I checked on 1.10.6 with a dummy <:Real that only defines round(x, ::RoundingMode), and floor(d), ceil(d), trunc(d) and round(d) all work — only the f(::Type{T}, d) variants throw. So what 1.11 actually adds are the generic trunc/floor/ceil/round(::Type{T}, x) fallbacks (base/rounding.jl:474-477), and IMO only those four need a fallback here:

# Base derives floor/ceil/trunc/round from `round(x, ::RoundingMode)`:
# https://docs.julialang.org/en/v1/manual/interfaces/#man-rounding-interface
Base.round(d::Dual, r::RoundingMode) = round(value(d), r)

# Julia 1.11 added the generic `f(::Type{T}, x)` fallbacks, so these can be
# dropped once 1.11 is the minimum supported version.
if VERSION < v"1.11"
    Base.floor(::Type{R}, d::Dual) where {R<:Real} = floor(R, value(d))
    Base.ceil(::Type{R}, d::Dual) where {R<:Real} = ceil(R, value(d))
    Base.trunc(::Type{R}, d::Dual) where {R<:Real} = trunc(R, value(d))
    Base.round(::Type{R}, d::Dual) where {R<:Real} = round(R, value(d))
end

Two advantages besides the smaller diff: the invalidations are fixed on the LTS as well (as written, 1.10 users keep all eight methods), and round(d, RoundUp) then works on all supported versions instead of only on 1.11+ — with the current version it would be a MethodError on 1.10, which seems like an unnecessary difference.

Also, AFAICT @static isn't needed since both branches only contain ordinary method definitions — a plain if is sufficient. (It would matter if the untaken branch used macros or bindings that don't exist on the other version.)

Could you add a few tests as well? The existing ones only cover Int and the no-type variants, so

  • the explicit rounding modes are untested, in particular RoundNearestTiesAway/RoundNearestTiesUp/RoundFromZero, which only become available with this PR, and
  • f(::Type{T}, d) with T<:AbstractFloat is untested. That one seems worth covering: round(::Type{T}, x, r) dispatches to Base._round_convert, which for T<:AbstractFloat compares x_t < x/x_t > x and calls signbit(x) on the original argument (base/float.jl:471) — i.e. on the Dual, not on value(d). It works, but only because we define the mixed comparisons, and it would be good to notice if that ever changes.

Something like

for r in (RoundDown, RoundUp, RoundToZero, RoundNearest,
          RoundNearestTiesAway, RoundNearestTiesUp, RoundFromZero)
    @test round(FDNUM, r) === round(PRIMAL, r)
    @test round(NESTED_FDNUM, r) === round(PRIMAL, r)
end
@test floor(Float32, FDNUM) === floor(Float32, PRIMAL)
@test ceil(Float32, FDNUM) === ceil(Float32, PRIMAL)

(The failures on pre are unrelated to this PR — they're JET bugs, fixed in JET 0.12. Our compat bound still caps at 0.11, so #827 is what actually resolves them; #828 additionally restricts the JET tests to explicitly supported versions.)

@JamesWrigley

Copy link
Copy Markdown
Contributor Author

Sure, changed the implementation and added those tests in cdafa99.

The other functions like floor/trunc are already implemented in terms of that
function in 1.11.
@devmotion
devmotion merged commit 569af35 into JuliaDiff:master Aug 6, 2026
21 of 27 checks passed
@JamesWrigley
JamesWrigley deleted the rounding branch August 6, 2026 10:20
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