Skip to content
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

Comparison with irrationals is not thread safe #52862

Open
OlivierHnt opened this issue Jan 11, 2024 · 2 comments
Open

Comparison with irrationals is not thread safe #52862

OlivierHnt opened this issue Jan 11, 2024 · 2 comments
Labels
bignums BigInt and BigFloat bug Indicates an unexpected problem or unintended behavior multithreading Base.Threads and related functionality

Comments

@OlivierHnt
Copy link
Contributor

OlivierHnt commented Jan 11, 2024

While investigating JuliaIntervals/IntervalArithmetic.jl#612, we found out that < hinges on setrounding which is not thread safe.

MWE:

julia> precision(BigFloat)
256

julia> x = BigFloat(1)
1.0

julia> Threads.@threads for _ in 1:100000
           x < π
       end

julia> precision(BigFloat)
288

This example was run using Julia v1.10

julia> versioninfo()
Julia Version 1.10.0
Commit 3120989f39b (2023-12-25 18:01 UTC)
Build Info:
  Official https://julialang.org/ release
Platform Info:
  OS: macOS (arm64-apple-darwin22.4.0)
  CPU: 8 × Apple M1
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-15.0.7 (ORCJIT, apple-m1)
  Threads: 11 on 4 virtual cores

The problem seems to be here

julia/base/irrationals.jl

Lines 99 to 101 in c5d7b87

<(x::AbstractIrrational, y::BigFloat) = setprecision(precision(y)+32) do
big(x) < y
end

cc @Kolaru and @dpsanders

@giordano giordano added bug Indicates an unexpected problem or unintended behavior multithreading Base.Threads and related functionality bignums BigInt and BigFloat labels Jan 11, 2024
@Joel-Dahne
Copy link

Joel-Dahne commented Jan 12, 2024

This problem is similar to #52859. At this place I think the fix is simple, just use specify the precision explicitly with BigFloat(x, precision = precision(y) + 32).

However, the construction of BigFloat from Irrational also uses setprecision and is hence not thread-safe. This is a harder problem to solve since when specifying irrationals one can give an arbitrary function for computing them. Here is the implementation

julia/base/irrationals.jl

Lines 212 to 240 in c5d7b87

function irrational(sym, val, def)
esym = esc(sym)
qsym = esc(Expr(:quote, sym))
bigconvert = isa(def,Symbol) ? quote
function Base.BigFloat(::Irrational{$qsym}, r::MPFR.MPFRRoundingMode=MPFR.ROUNDING_MODE[]; precision=precision(BigFloat))
c = BigFloat(;precision=precision)
ccall(($(string("mpfr_const_", def)), :libmpfr),
Cint, (Ref{BigFloat}, MPFR.MPFRRoundingMode), c, r)
return c
end
end : quote
function Base.BigFloat(::Irrational{$qsym}; precision=precision(BigFloat))
setprecision(BigFloat, precision) do
$(esc(def))
end
end
end
quote
const $esym = Irrational{$qsym}()
$bigconvert
let v = $val, v64 = Float64(v), v32 = Float32(v)
Base.Float64(::Irrational{$qsym}) = v64
Base.Float32(::Irrational{$qsym}) = v32
end
@assert isa(big($esym), BigFloat)
@assert Float64($esym) == Float64(big($esym))
@assert Float32($esym) == Float32(big($esym))
end
end

@Joel-Dahne
Copy link

In fact setprecision is used in a non-safe way in three more places in that file. For construction of Rational{T] as well as construction of Float32 and Float64 with rounding. In all of those places it would be fine to replace a constructor that explicitly specifies the precision though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bignums BigInt and BigFloat bug Indicates an unexpected problem or unintended behavior multithreading Base.Threads and related functionality
Projects
None yet
Development

No branches or pull requests

3 participants