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

Test fails on Julia master (1.4) #6

Closed
KristofferC opened this issue Jan 3, 2020 · 4 comments
Closed

Test fails on Julia master (1.4) #6

KristofferC opened this issue Jan 3, 2020 · 4 comments

Comments

@KristofferC
Copy link

Some of the tests fail with Julia master (1.4):

@count_ops: Test Failed at /Users/kristoffer/JuliaPkgEval/GFlops.jl/test/runtests.jl:71
  Expression: cnt.mul64 == 100
   Evaluated: 0 == 100
Stacktrace:
 [1] top-level scope at /Users/kristoffer/JuliaPkgEval/GFlops.jl/test/runtests.jl:71
 [2] top-level scope at /Users/kristoffer/julia/usr/share/julia/stdlib/v1.5/Test/src/Test.jl:1116
 [3] top-level scope at /Users/kristoffer/JuliaPkgEval/GFlops.jl/test/runtests.jl:57
 [4] top-level scope at /Users/kristoffer/julia/usr/share/julia/stdlib/v1.5/Test/src/Test.jl:1116
 [5] top-level scope at /Users/kristoffer/JuliaPkgEval/GFlops.jl/test/runtests.jl:35
@count_ops: Test Failed at /Users/kristoffer/JuliaPkgEval/GFlops.jl/test/runtests.jl:72
  Expression: GFlops.flop(cnt) == 200
   Evaluated: 100 == 200
Stacktrace:
 [1] top-level scope at /Users/kristoffer/JuliaPkgEval/GFlops.jl/test/runtests.jl:72
 [2] top-level scope at /Users/kristoffer/julia/usr/share/julia/stdlib/v1.5/Test/src/Test.jl:1116
 [3] top-level scope at /Users/kristoffer/JuliaPkgEval/GFlops.jl/test/runtests.jl:57
 [4] top-level scope at /Users/kristoffer/julia/usr/share/julia/stdlib/v1.5/Test/src/Test.jl:1116
 [5] top-level scope at /Users/kristoffer/JuliaPkgEval/GFlops.jl/test/runtests.jl:35
  100.00 GFlops,  75.57% peak  (2.00e+02 flop, 2.00e-09 s)
  50.00 GFlops,  33.47% peak  (1.00e+02 flop, 2.00e-09 s)
@gflops: Test Failed at /Users/kristoffer/JuliaPkgEval/GFlops.jl/test/runtests.jl:104
  Expression: #= /Users/kristoffer/JuliaPkgEval/GFlops.jl/test/runtests.jl:104 =# @gflops(my_axpy!(π, $(rand(N)), y)) == N
   Evaluated: 50.0 == 100
Stacktrace:
 [1] top-level scope at /Users/kristoffer/JuliaPkgEval/GFlops.jl/test/runtests.jl:104
 [2] top-level scope at /Users/kristoffer/julia/usr/share/julia/stdlib/v1.5/Test/src/Test.jl:1116
 [3] top-level scope at /Users/kristoffer/JuliaPkgEval/GFlops.jl/test/runtests.jl:97
 [4] top-level scope at /Users/kristoffer/julia/usr/share/julia/stdlib/v1.5/Test/src/Test.jl:1116
 [5] top-level scope at /Users/kristoffer/JuliaPkgEval/GFlops.jl/test/runtests.jl:35
  10000.00 GFlops,  6610.24% peak  (2.00e+04 flop, 2.00e-09 s)
Test Summary: | Pass  Fail  Total

It would be good if we could figure out the root cause of this to see if it is a regression in Julia or what is going on.

@ffevotte
Copy link
Member

ffevotte commented Jan 3, 2020

Thanks for bringing this up!

It looks like this has to do with the way irrational constants like pi are handled. Supposing x isa Float64 and performing the pi*x product, here is a list of all operations as seen by Cassette on julia 1.3:

(*, Irrational{:π}, Float64)
(promote, Irrational{:π}, Float64)
(Base._promote, Irrational{:π}, Float64)
(promote_type, DataType, DataType)
(promote_rule, DataType, DataType)
(promote_type, DataType, DataType)
(promote_rule, DataType, DataType)
(promote_type, DataType, Core.TypeofBottom)
(convert, DataType, Irrational{:π})
(convert, DataType, Float64)
(tuple, Float64, Float64)
(Base.indexed_iterate, Tuple{Float64,Float64}, Int64)
(getfield, Tuple{Float64,Float64}, Int64)
(+, Int64, Int64)
(add_int, Int64, Int64)
(tuple, Float64, Int64)
(getfield, Tuple{Float64,Int64}, Int64)
(getfield, Tuple{Float64,Int64}, Int64)
(getfield, Tuple{Float64,Float64}, Int64)
(+, Int64, Int64)
(add_int, Int64, Int64)
(tuple, Float64, Int64)
(getfield, Tuple{Float64,Int64}, Int64)
(tuple, Irrational{:π}, Float64)
(tuple, Float64, Float64)
(Base.not_sametype, Tuple{Irrational{:π},Float64}, Tuple{Float64,Float64})
(tuple, Float64, Float64)
(*, Float64, Float64)
(mul_float, Float64, Float64)

As expected, the initial *(::Irrational, ::Float64) call internally converts pi to a Float64, then calls the *(::Float64, ::Float64) method, ultimately performing a mul_float (which GFlops counts)

With julia nightly, things start in the same way, but the *(::Float64, ::Float64) is never called:

(*, Irrational{:π}, Float64)
(promote, Irrational{:π}, Float64)
(Base._promote, Irrational{:π}, Float64)
(promote_type, DataType, DataType)
(promote_rule, DataType, DataType)
(promote_type, DataType, DataType)
(promote_rule, DataType, DataType)
(promote_type, DataType, Core.TypeofBottom)
(convert, DataType, Irrational{:π})
(convert, DataType, Float64)
(tuple, Float64, Float64)
(Base.indexed_iterate, Tuple{Float64,Float64}, Int64)
(getfield, Tuple{Float64,Float64}, Int64)
(+, Int64, Int64)
(Core.Intrinsics.add_int, Int64, Int64)
(tuple, Float64, Int64)
(getfield, Tuple{Float64,Int64}, Int64)
(getfield, Tuple{Float64,Int64}, Int64)
(getfield, Tuple{Float64,Float64}, Int64)
(+, Int64, Int64)
(Core.Intrinsics.add_int, Int64, Int64)
(tuple, Float64, Int64)
(getfield, Tuple{Float64,Int64}, Int64)
(tuple, Irrational{:π}, Float64)
(tuple, Float64, Float64)
(Base.not_sametype, Tuple{Irrational{:π},Float64}, Tuple{Float64,Float64})
(tuple, Float64, Float64)

I'm going to investigate further, but here are my thoughts so far:

  1. in the worst case scenario, we could change the test cases so that they don't use irrational numbers anymore. This would be a bit of a regression in GFlops in that operations involving a constant like pi would not be counted anymore, but we could probably live with that...
  2. the change in behavior shown above is detected using Cassette, so that an ideal fix for this bug might involve the sources of Julia itself, GFlops.jl or Cassette.jl.

Would you by any chance happen to know whether Julia-1.4 introduces subtle changes in the way Irrational constants are handled internally?

@ffevotte
Copy link
Member

ffevotte commented Jan 3, 2020

FWIW, here is the Cassette-based code I used to produce the call traces above:

using Cassette
Cassette.@context Ctx;

function Cassette.prehook(ctx::Ctx,
                          op::Any,
                          a::T1, b::T2) where {T1, T2}
    println((op, T1, T2))
end

ctx = Ctx()
Cassette.overdub(ctx, ()->pi*2.0)

@KristofferC
Copy link
Author

I stepped through it with Debugger and it gives the same instructions executed until

About to run: (tuple)(3.141592653589793, 2.0)
1|debug>
In promote(x, y) at promotion.jl:281
 280  function promote(x, y)
 281      @_inline_meta
 282      px, py = _promote(x, y)
 283      not_sametype((x,y), (px,py))
>284      px, py
 285  end

About to run: return (3.141592653589793, 2.0)
1|debug>
In *(x, y) at promotion.jl:312
>312  *(x::Number, y::Number) = *(promote(x,y)...)

###################
# Cassette stops here  # 
###################

About to run: (Core._apply_iterate)(iterate, *, (3.141592653589793, 2.0))
1|debug>
In *(x, y) at float.jl:405
>405  *(x::Float64, y::Float64) = mul_float(x, y)

About to run: (Core.Intrinsics.mul_float)(3.141592653589793, 2.0)
1|debug>
In *(x, y) at float.jl:405
>405  *(x::Float64, y::Float64) = mul_float(x, y)

About to run: return 6.283185307179586
1|debug>
In *(x, y) at promotion.jl:312
>312  *(x::Number, y::Number) = *(promote(x,y)...)

About to run: return 6.283185307179586
1|debug>
6.283185307179586

So the problem is that Cassette doesn't recurse through into _apply_iterate. Will file a bug with Cassette.

@ffevotte
Copy link
Member

ffevotte commented Jan 3, 2020

Thanks!

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

No branches or pull requests

2 participants