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

fld1 is weird #28973

Closed
jleugeri opened this issue Aug 30, 2018 · 7 comments · Fixed by #46938
Closed

fld1 is weird #28973

jleugeri opened this issue Aug 30, 2018 · 7 comments · Fixed by #46938
Labels
deprecation This change introduces or involves a deprecation maths Mathematical functions needs decision A decision on this change is needed

Comments

@jleugeri
Copy link

Hi.

I necro-posted this as a comment in a different issue ( #14487 ), which is open since 2015 and thus probably won't get looked at any time soon. So here goes. I was playing around with fld1 on Julia 1.0 on a Ubuntu system, delved too deep and awoke something in Khazad-dum.

fld and cld work as expected:

julia> fld(0.09, 0.1)
0.0
julia> fld(0.08, 0.1)
0.0
julia> fld(0.07, 0.1)
0.0
julia> fld(0.06, 0.1)
0.0

and

julia> cld(0.09, 0.1)
1.0
julia> cld(0.08, 0.1)
1.0
julia> cld(0.07, 0.1)
1.0
julia> cld(0.06, 0.1)
1.0

However:

julia> fld1(0.09, 0.1)
1.0
julia> fld1(0.08, 0.1)
0.0
julia> fld1(0.07, 0.1)
1.0
julia> fld1(0.06, 0.1)
1.0

WAT.

My system:

Julia Version 1.0.0
Commit 5d4eaca0c9 (2018-08-08 20:58 UTC)
Platform Info:
  OS: Linux (x86_64-pc-linux-gnu)
      Ubuntu 16.04.5 LTS
  uname: Linux 4.4.0-127-generic #153-Ubuntu SMP Sat May 19 10:58:46 UTC 2018 x86_64 x86_64
  CPU: Intel(R) Core(TM) i7-2600 CPU @ 3.40GHz: 
              speed         user         nice          sys         idle          irq
       #1  2998 MHz   29220504 s  374401370 s   16316182 s  412409612 s          0 s
       #2  3495 MHz   25098103 s  389922335 s    8533915 s  413120981 s          0 s
       #3  2506 MHz   32293244 s  397619253 s    7969477 s  399074571 s          0 s
       #4  2546 MHz   27990786 s  380869437 s    7981724 s  420045227 s          0 s
       #5  3026 MHz   19610614 s  405248460 s    6253806 s  406291328 s          0 s
       #6  3500 MHz   24186996 s  369161728 s    7044432 s  436747971 s          0 s
       #7  2207 MHz   25446131 s  399302571 s   10678770 s  402065306 s          0 s
       #8  1660 MHz   24326246 s  391760285 s   12716558 s  408596497 s          0 s
       
  Memory: 31.378002166748047 GB (3988.6875 MB free)
  Uptime: 8.380479e6 sec
  Load Avg:  0.2607421875  0.46875  0.4658203125
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-6.0.0 (ORCJIT, sandybridge)

Can anyone else reproduce this, or is this a problem uniquely on my system? I could easily live without fld1, but this result ... makes me nervous.

@simonbyrne
Copy link
Contributor

We could just get rid of it? fld1 and fldmod1 aren't actually called anywhere in base or the stdlib.

@simonbyrne
Copy link
Contributor

alternatively, we could make it only defined for integer values (it is somewhat non-sensical for floating point).

@JeffreySarnoff
Copy link
Contributor

JeffreySarnoff commented Aug 30, 2018

julia> vals = (collect(0.09:-0.01:0.06)...,)
(0.09, 0.08, 0.07, 0.06)

julia> xs=vals;ys=(0.1,0.1,0.1,0.1);

julia> # from def of mod1, fld1: x == (fld1(x, y) - 1) * y + mod1(x, y)

julia> (((fld1.(xs,ys)) .- 1) .* ys) .+ mod1.(xs,ys)
(0.09, -0.020000000000000004, 0.07, 0.06)

@JeffreySarnoff
Copy link
Contributor

In calendar calculations, amod [adjusted modulus] is useful:

function amod( x::T , y::T ) where {T<:Signed}
    return y + modulus(x, -y)
end

function modulus( x::T, y::T ) where {T<:Signed}
     return x - y * floor(T, x / y)
end

amod(x::T, y::T) = modulus(x::T, y::T) === zero(T) ? y : modulus(x, y)

@JeffBezanson JeffBezanson added maths Mathematical functions needs decision A decision on this change is needed deprecation This change introduces or involves a deprecation labels Sep 4, 2018
@devmotion
Copy link
Contributor

Just ran into this:

julia> fld1(0.4, 0.9)
1.0

julia> fld1(0.41, 0.9)
1.0

julia> fld1(0.401, 0.9)
1.0

julia> fld1(0.4001, 0.9)
1.0

julia> fld1(0.40001, 0.9)
0.0

That's definitely not what I expected.

@JeffreySarnoff
Copy link
Contributor

JeffreySarnoff commented Sep 27, 2022

😢
[from Base, v1.9.0-DEV.1401]

fld1(x::T, y::T) where {T<:Real} = (m = mod1(x, y); fld(x + y - m, y))
fld(x, y) = div(a, b, RoundDown)
julia> x1 = 0.400_100; x2 = 0.400_010; y = 0.9;

julia> mod1(x1, y), mod1(x2, y)
(0.4001, 0.40001)

julia> fld1(x1, y), fld1(x2, y)
(1.0, 0.0)

# the bug is here
julia> a = x1 + y - x1; b = y; a, div(a, b, RoundDown)
(0.9, 1.0)

julia> a = x2 + y - x2; b = y; a, div(a, b, RoundDown)
(0.8999999999999999, 0.0)

# redefining fld1 quashes that bug

Base.fld1(x::T, y::T) where {T<:Real} = (m = mod1(x, y); fld((x - m) + y, y))

I am preparing a PR

@devmotion
Copy link
Contributor

It seems that would also fix the example in the OP:

julia> Base.fld1(x::T, y::T) where {T<:Real} = (m = mod1(x, y); fld((x - m) + y, y))

julia> fld1(0.09, 0.1)
1.0

julia> fld1(0.08, 0.1)
1.0

julia> fld1(0.07, 0.1)
1.0

julia> fld1(0.06, 0.1)
1.0

JeffreySarnoff added a commit to JeffreySarnoff/julia that referenced this issue Sep 28, 2022
JeffreySarnoff added a commit to JeffreySarnoff/julia that referenced this issue Sep 29, 2022
oscardssmith pushed a commit that referenced this issue Oct 3, 2022
* bugfix: fld1 order of ops

fixes #28973
KristofferC pushed a commit that referenced this issue Oct 27, 2022
* bugfix: fld1 order of ops

fixes #28973

(cherry picked from commit fcdc5bc)
KristofferC pushed a commit that referenced this issue Oct 28, 2022
* bugfix: fld1 order of ops

fixes #28973

(cherry picked from commit fcdc5bc)
KristofferC pushed a commit that referenced this issue Oct 28, 2022
* bugfix: fld1 order of ops

fixes #28973

(cherry picked from commit fcdc5bc)
KristofferC pushed a commit that referenced this issue Dec 21, 2022
* bugfix: fld1 order of ops

fixes #28973

(cherry picked from commit fcdc5bc)
KristofferC pushed a commit that referenced this issue Dec 21, 2022
* bugfix: fld1 order of ops

fixes #28973

(cherry picked from commit fcdc5bc)
KristofferC pushed a commit that referenced this issue Dec 21, 2022
* bugfix: fld1 order of ops

fixes #28973

(cherry picked from commit fcdc5bc)
KristofferC pushed a commit that referenced this issue Dec 21, 2022
* bugfix: fld1 order of ops

fixes #28973

(cherry picked from commit fcdc5bc)
staticfloat pushed a commit that referenced this issue Dec 23, 2022
* bugfix: fld1 order of ops

fixes #28973

(cherry picked from commit fcdc5bc)
KristofferC pushed a commit that referenced this issue Oct 10, 2023
* bugfix: fld1 order of ops

fixes #28973

(cherry picked from commit fcdc5bc)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deprecation This change introduces or involves a deprecation maths Mathematical functions needs decision A decision on this change is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants