-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Flag for enabling warnings when converting from AbstractFloat
#138
Comments
I actually like the idea! how about just using Example: function Base.convert(::Type{Arb}, x::Union{Float16, Float32, Float64})
@debug "Converting a float to Arb. This may result in loss of precision!"
return Arb(x)
end and in REPL:
Of course this will not save the user from doing |
A
My current feeling is that we should update the setter. It's better to give the user false positives than to miss some true positives, in particular when working with computer assisted proofs. Another question is which types to warn for. For sure I'll experiment a bit with this locally and see what I like! |
Setter for On the other hand I sometimes purposely create arbs from whole matrices of |
I just noticed quite severe regression: julia> ENV["JULIA_DEBUG"]=""
""
julia> @btime 1.0*$(Arb(2.6)) # @debug statement in `convert`
108.709 ns (2 allocations: 128 bytes)
2.6000000000000000888178419700125232338905334472656250000000000000000000000000
julia> @btime 1.0*$(Arb(2.6)) # no @debug statement in `convert`
39.899 ns (2 allocations: 128 bytes)
2.6000000000000000888178419700125232338905334472656250000000000000000000000000
which isn't very surprising: I think const _debug = Ref(false)
function Base.convert(::Type{Arb}, x::Union{Float16, Float32, Float64})
_debug[] && @warn "Converting a float to Arb. This may result in loss of precision!"
return Arb(x)
end which is more digestible
|
I agree! For me it is also very common to construct some sort of approximation from |
The performance regression is worrying, I didn't expect |
the solution with const _debug = Ref(true)
#Arb
function Arb(x::Any; prec::Integer = _precision(x))
_debug[] && @warn "Converting $(typeof(x)) to Arb. This may result in loss of precision!"
return set!(Arb(prec = prec), x)
end
function Arb(x::T; prec::Integer = _precision(x)) where T <: Union{Rational, Integer, ArbLike, ArfLike}
return set!(Arb(prec = prec), x)
end julia> 2.6*Arb(2.6)
[6.7600000000000004618527782440651287048398261358993041172856528278622967320644 +/- 4.90e-77]
julia> Arblib._debug[] = true
true
julia> 2.6*Arb(2.6)
┌ Warning: Converting Float64 to Arb. This may result in loss of precision!
└ @ Arblib ~/.julia/dev/Arblib/src/constructors.jl:16
┌ Warning: Converting Float64 to Arb. This may result in loss of precision!
└ @ Arblib ~/.julia/dev/Arblib/src/constructors.jl:16
[6.7600000000000004618527782440651287048398261358993041172856528278622967320644 +/- 4.90e-77]
julia> Arblib._debug[] = false
false
julia> @btime 2.6*$(Arb(2.6))
41.764 ns (2 allocations: 128 bytes)
[6.7600000000000004618527782440651287048398261358993041172856528278622967320644 +/- 4.90e-77] without the julia> Arblib._debug[] = true
true
julia> 2.6*Arb(2.6)
[6.7600000000000004618527782440651287048398261358993041172856528278622967320644 +/- 4.90e-77]
julia> Arblib._debug[] = true
true
julia> @btime 2.6*$(Arb(2.6))
40.423 ns (2 allocations: 128 bytes)
[6.7600000000000004618527782440651287048398261358993041172856528278622967320644 +/- 4.90e-77]
julia> 2.6*Arb(2.6)
[6.7600000000000004618527782440651287048398261358993041172856528278622967320644 +/- 4.90e-77]
julia> @btime 2.6*$(Arb(2.6))
39.929 ns (2 allocations: 128 bytes)
[6.7600000000000004618527782440651287048398261358993041172856528278622967320644 +/- 4.90e-77] so maybe we can pay this ~1-2ns price? Especially as branch prediction should do a pretty good job here :) |
Okay, lets start with this since it's the simpler solution! I'll do some local testing and try it out for a while before pushing anything probably. I agree that this should be a trivial job for the branch predictor! |
An alternative with zero overhead would be to a function instead of a julia> using Arblib, BenchmarkTools
julia> enable_debug() = false
enable_debug (generic function with 1 method)
julia> function MyArb(x; prec::Integer = Arblib._precision(x))
enable_debug() && @warn "Converting $(typeof(x)) to Arb. This may result in loss of precision!"
return Arblib.set!(Arb(prec = prec), x)
end
MyArb (generic function with 1 method)
julia> x = 2.6;
julia> y = Arb(x);
julia> @btime $x * $y;
56.531 ns (2 allocations: 128 bytes)
julia> @btime Arb($x) * $y;
56.727 ns (2 allocations: 128 bytes)
julia> @btime MyArb($x) * $y;
56.654 ns (2 allocations: 128 bytes)
julia> enable_debug() = true # causes all dependent methods to be recompiled
enable_debug (generic function with 1 method)
julia> MyArb(x) * y
┌ Warning: Converting Float64 to Arb. This may result in loss of precision!
└ @ Main REPL[27]:2
[6.7600000000000004618527782440651287048398261358993041172856528278622967320644 +/- 4.90e-77] This is approach is e.g. also used by TimerOutputs (https://github.com/KristofferC/TimerOutputs.jl#overhead). In comparison, the julia> const _debug = Ref(false);
julia> function MyArb(x; prec::Integer = Arblib._precision(x))
_debug[] && @warn "Converting $(typeof(x)) to Arb. This may result in loss of precision!"
return Arblib.set!(Arb(prec = prec), x)
end
MyArb (generic function with 1 method)
julia> @btime MyArb($x) * $y;
59.130 ns (2 allocations: 128 bytes) Not sure how important the additional performance is for you 🙂 I can make a PR with an |
That looks like a great idea! If I understand it correctly it would mean that you pay the cost of recompilation when switching the flag but otherwise it should be zero overheard since the check would be optimized out? If you could make a PR that would be amazing! The reason that it has not been moving forward for a while is because some other parts of my current project took longer than expected so I have not yet gotten to checking all the code, but eventually I'll have to do this. The main question that remains to answer is exactly what methods to add this check to. This is likely problem dependent but I think adding it to all |
Yes, exactly. A simple example that illustrates how it is optimized and recompiled by the compiler: julia> enable_debug() = true
enable_debug (generic function with 1 method)
julia> f() = enable_debug() ? 1 : 1.0
f (generic function with 1 method)
julia> @code_llvm f()
; @ REPL[2]:1 within `f'
define i64 @julia_f_256() {
top:
ret i64 1
}
julia> enable_debug() = false
enable_debug (generic function with 1 method)
julia> @code_llvm f()
; @ REPL[2]:1 within `f'
define double @julia_f_277() {
top:
ret double 1.000000e+00
} |
The same with julia> const debug = Ref(true)
Base.RefValue{Bool}(true)
julia> g() = debug[] ? 1 : 1.0
g (generic function with 1 method)
julia> @code_llvm g()
; @ REPL[7]:1 within `g'
define { {}*, i8 } @julia_g_290([8 x i8]* noalias nocapture align 8 dereferenceable(8) %0) {
top:
; ┌ @ refvalue.jl:56 within `getindex'
; │┌ @ Base.jl:33 within `getproperty'
%1 = load i8, i8* inttoptr (i64 139923040448560 to i8*), align 16
%2 = and i8 %1, 1
%.not = icmp eq i8 %2, 0
; └└
%spec.select = select i1 %.not, { {}*, i8 } { {}* inttoptr (i64 139923021809568 to {}*), i8 -127 }, { {}*, i8 } { {}* inttoptr (i64 139923014266976 to {}*), i8 -126 }
ret { {}*, i8 } %spec.select
}
julia> @code_warntype g()
Variables
#self#::Core.Const(g)
Body::Union{Float64, Int64}
1 ─ %1 = Base.getindex(Main.debug)::Bool
└── goto #3 if not %1
2 ─ return 1
3 ─ return 1.0
julia> debug[] = false
false
julia> @code_llvm g()
; @ REPL[7]:1 within `g'
define { {}*, i8 } @julia_g_427([8 x i8]* noalias nocapture align 8 dereferenceable(8) %0) {
top:
; ┌ @ refvalue.jl:56 within `getindex'
; │┌ @ Base.jl:33 within `getproperty'
%1 = load i8, i8* inttoptr (i64 139923040448560 to i8*), align 16
%2 = and i8 %1, 1
%.not = icmp eq i8 %2, 0
; └└
%spec.select = select i1 %.not, { {}*, i8 } { {}* inttoptr (i64 139923021809568 to {}*), i8 -127 }, { {}*, i8 } { {}* inttoptr (i64 139923014266976 to {}*), i8 -126 }
ret { {}*, i8 } %spec.select
}
julia> @code_warntype g()
Variables
#self#::Core.Const(g)
Body::Union{Float64, Int64}
1 ─ %1 = Base.getindex(Main.debug)::Bool
└── goto #3 if not %1
2 ─ return 1
3 ─ return 1.0 |
Great! Then we get truly zero overhead when the debugging is disabled! |
Previously the error_on_failure argument for fpwrap methods would default to false. Now it still defaults to false but this can be changed using the fpwrap_error_on_fauilure_default_set method. This allows switching this on or off globally for easier debugging whens searching for what produces a NaN. The current implementation leads to zero overhead, it only incurs a compile time cost. See comments in #138 and #150 for some context.
I find that one of the easiest mistakes to make when working with
Arblib
in Julia is to accidentally useFloat64
for some internal computations. For example forx::Arb
it's perfectly fine to do any of the followingBut you get a wrong result for, e.g.,
One of the common things for all of the wrong examples is that they involve a conversion from
AbstractFloat
toArb
. The Idea I have had is to have some sort of flag or macro which, when enable, prints a warning whenever aAbstractFloat
is converted toArb
. So it would behave something likeDoes this sound interesting to you?
I don't know exactly how this would be implemented, I feel like it should be possible but I don't know exactly how. Any thoughts?
The text was updated successfully, but these errors were encountered: