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

Strange TypeError on master #37229

Closed
KristofferC opened this issue Aug 27, 2020 · 13 comments · Fixed by #37370
Closed

Strange TypeError on master #37229

KristofferC opened this issue Aug 27, 2020 · 13 comments · Fixed by #37370
Labels
bug Indicates an unexpected problem or unintended behavior regression Regression in behavior compared to a previous version
Milestone

Comments

@KristofferC
Copy link
Member

KristofferC commented Aug 27, 2020

Repro:

using GeoGreensFunctions

xs = collect(-10.0: 5.0: 10.0)
ys = collect(-8.0: 5.0: 8.0)
zs = 4.0

xyz = Iterators.product(zs, ys, xs) |> collect
Y = [q[2] for q in xyz] |> vec
X = [q[3] for q in xyz] |> vec
Z = [q[1] for q in xyz] |> vec

stress_rect_fs(X, Y, Z, 3.0, 1.0, 0.0, 2.0, 1.0, 10.0, 75.0, 30.0, 20.0, 1.0, 0.15, 0.66e11, 0.33e11, Val(:pc))

gives

ERROR: TypeError: in new, expected Tuple{Vector{Float64},Vector{Float64},Vector{Float64}}, got a value of type 
                                   Tuple{Vector{Float64},Vector{Float64},Vector{Float64}}
Stacktrace:
 [1] hcat
   @ ./array.jl:1668 [inlined]
 [2] strain_rect_fs(x::Vector{Float64}, y::Vector{Float64}, z::Vector{Float64}, x₀::Float64, y₀::Float64, depth::Float64, L::Float64, W::Float64, plunge::Float64, dip::Float64, strike::Float64, rake::Float64, slip::Float64, opening::Float64, λ::Float64, μ::Float64, ref::Val{:pc})

This seems to be some compilation error because it runs fine in the debugger.

Note that this function takes a long time to compile (order of a minute).

@KristofferC KristofferC added bug Indicates an unexpected problem or unintended behavior regression Regression in behavior compared to a previous version labels Aug 27, 2020
@KristofferC KristofferC added this to the 1.6 features milestone Aug 27, 2020
@martinholters
Copy link
Member

Tried to reproduce on master (1.6.0-DEV.772 2020-08-30 41e603e) and got

julia> strain_rect_fs(x, y, z, x₀, y₀, depth, L, W, plunge, dip, strike, rake, slip, opening, λ, μ, ref)
ERROR: BoundsError: attempt to access 5-element Vector{Float64} at index [Bool[0, 0, 0, 1]]
Stacktrace:
 [1] throw_boundserror(A::Vector{Float64}, I::Tuple{Base.LogicalIndex{Int64, BitVector}})
   @ Base ./abstractarray.jl:600
 [2] checkbounds
   @ ./abstractarray.jl:565 [inlined]
 [3] _getindex
   @ ./multidimensional.jl:745 [inlined]
 [4] getindex
   @ ./abstractarray.jl:1119 [inlined]
 [5] strain_rect_fs(x::Vector{Float64}, y::Vector{Float64}, z::Vector{Float64}, x₀::Float64, y₀::Float64, depth::Float64, L::Float64, W::Float64, plunge::Float64, dip::Float64, strike::Float64, rake::Float64, slip::Float64, opening::Float64, λ::Float64, μ::Float64, ref::Val{:pc})
   @ GeoGreensFunctions ~/.julia/packages/GeoGreensFunctions/XI27d/src/funcs/disl_rect.jl:211
 [6] top-level scope
   @ REPL[29]:1

Can't tell whether that error is legit, but at least it's not so obviously bizarre as the TypeError you got.

@KristofferC
Copy link
Member Author

KristofferC commented Sep 1, 2020

Hm, interesting, I have to double-check that I didn't mess up my repro. I seem to get that error as well now but I remember that I could run it in the interpreter and get a reasonable result..

@KristofferC
Copy link
Member Author

KristofferC commented Sep 1, 2020

I think I indeed botched my repro. I updated the first post. Now I get:

julia> stress_rect_fs(X, Y, Z, 3.0, 1.0, 0.0, 2.0, 1.0, 10.0, 75.0, 30.0, 20.0, 1.0, 0.15, 0.66e11, 0.33e11, Val(:pc))
ERROR: TypeError: in new, expected Tuple{Vector{Float64}, Vector{Float64}, Vector{Float64}}, got a value of type Tuple{Vector{Float64}, Vector{Float64}, Vector{Float64}}
Stacktrace:
 [1] hcat
   @ ./array.jl:1668 [inlined]
 [2] strain_rect_fs(x::Vector{Float64}, y::Vector{Float64}, z::Vector{Float64}, x₀::Float64, y₀::Float64, depth::Float64, L::Float64, W::Float64, plunge::Float64, dip::Float64, strike::Float64, rake::Float64, slip::Float64, opening::Float64, λ::Float64, μ::Float64, ref::Val{:pc})
   @ GeoGreensFunctions ~/PkgEval16/GeoGreensFunctions.jl/src/funcs/disl_rect.jl:185

julia> using JuliaInterpreter

julia> @interpret stress_rect_fs(X, Y, Z, 3.0, 1.0, 0.0, 2.0, 1.0, 10.0, 75.0, 30.0, 20.0, 1.0, 0.15, 0.66e11, 0.33e11, Val(:pc))
([-843394.6565703829, -5.384947085022198e6, -4.260118397345182e6, 2.2635914219815093e6, 3.430562459278418e6, -4.0030115433611
...

@martinholters
Copy link
Member

Somewhat reduced, self-contained reproducer:

@inline function RDSetupS()
    RDVertex = zeros(3)
    SideVec = zeros(3)
    x, y, z = Float64[], Float64[], Float64[]
    y1 = @. SideVec[3] * (y - RDVertex[2]) - SideVec[2] * (z - RDVertex[3])
    z1 = @. SideVec[2] * (y - RDVertex[2]) + SideVec[3] * (z - RDVertex[3])

    sinA, cosA = -1.0, 0.0
    eta = @. y * cosA - z * sinA
    zeta = @. y * sinA + z * cosA
    x2 = @. x ^ 2
    y2 = @. y ^ 2
    z2 = @. z ^ 2
    r2 = @. x2 + y2 + z2
    r = @. (r2) # use `hypot` for higher precision
    r3 = @. r2 * r
    rz = @. r * (r - z)
    r2z2 = @. rz ^ 2
    r3z = @. r3 * (r - z)
    W = @. zeta - r
    W2 = @. W ^ 2
    Wr = @. W * r
    W2r = @. W2 * r
    Wr3 = @. W * r3
    W2r2 = @. W2 * r2
    C = @. (r * cosA - z) / Wr
    S = @. (r * sinA - y) / Wr

    rFi_rx = @. (eta / r / (r - zeta) - y / r / (r - z)) / 4 / π
    rFi_ry = @. (x / r / (r - z) - cosA * x / r / (r - zeta)) / 4 / π
    rFi_rz = @. (sinA * x / r / (r - zeta)) / 4 / π

    @. (0.15 * (rFi_rx) + 0.15 / 8 / π / (1 - 0.33) * (eta / Wr + eta * x2 / W2r2 - eta * x2 / Wr3 + y / rz -
        x2 * y / r2z2 - x2 * y / r3z) - 0.0 * x / 8 / π / (1 - 0.33) * (((2 * 0.33 + 1) / Wr + x2 / W2r2 - x2 / Wr3) * cosA +
        (2 * 0.33 + 1) / rz - x2 / r2z2 - x2 / r3z) + 0.0 * x * sinA / 8 / π / (1 - 0.33) * ((2 * 0.33 + 1) / Wr + x2 / W2r2 - x2 / Wr3))

    @. (0.0 * (rFi_ry) +
        0.15 / 8 / π / (1 - 0.33) * ((1 / Wr + S ^ 2 - y2 / Wr3) * eta + (2 * 0.33 + 1) * y / rz - y ^ 3 / r2z2 -
        y ^ 3 / r3z - 2 * 0.33 * cosA * S) -
        0.0 * x / 8 / π / (1 - 0.33) * (1 / rz - y2 / r2z2 - y2 / r3z +
        (1 / Wr + S ^ 2 - y2 / Wr3) * cosA) +
        0.0 * x * sinA / 8 / π / (1 - 0.33) * (1 / Wr + S ^ 2 - y2 / Wr3))

    @. (0.0 * (rFi_rz) +
        0.15 / 8 / π / (1 - 0.33) * (eta / W / r + eta * C ^ 2 - eta * z2 / Wr3 + y * z / r3 +
        2 * 0.33 * sinA * C) -
        0.0 * x / 8 / π / (1 - 0.33) * ((1 / Wr + C ^ 2 - z2 / Wr3) * cosA + z / r3) +
        0.0 * x * sinA / 8 / π / (1 - 0.33) * (1 / Wr + C ^ 2 - z2 / Wr3))

    @. (0.15 * (rFi_ry) / 2 + 0.0 * (rFi_rx) / 2 -
        0.15 / 8 / π / (1 - 0.33) * (x * y2 / r2z2 - 0.33 * x / rz + x * y2 / r3z - 0.33 * x * cosA / Wr +
        eta * x * S / Wr + eta * x * y / Wr3) +
        0.0 / 8 / π / (1 - 0.33) * (x2 * y / r2z2 - 0.33 * y / rz + x2 * y / r3z + 0.33 * cosA * S +
        x2 * y * cosA / Wr3 + x2 * cosA * S / Wr) -
        0.0 * sinA / 8 / π / (1 - 0.33) * (0.33 * S + x2 * S / Wr + x2 * y / Wr3))

    @. (0.15 * (rFi_rz) / 2 + 0.0 * (rFi_rx) / 2 -
        0.15 / 8 / π / (1 - 0.33) * (-x * y / r3 + 0.33 * x * sinA / Wr + eta * x * C / Wr +
        eta * x * z / Wr3) +
        0.0 / 8 / π / (1 - 0.33) * (-x2 / r3 + 0.33 / r + 0.33 * cosA * C + x2 * z * cosA / Wr3 +
        x2 * cosA * C / Wr) -
        0.0 * sinA / 8 / π / (1 - 0.33) * (0.33 * C + x2 * C / Wr + x2 * z / Wr3))
end

function strain_rect_fs()
    hcat(zeros(1))

    c = falses(1)
    if any(c)
        RDSetupS()
        RDSetupS()
        RDSetupS()
        RDSetupS()
        RDSetupS()
        RDSetupS()
    end
end

function stress_rect_fs()
    strain_rect_fs()
end

stress_rect_fs()

That gives

ERROR: TypeError: in new, expected Tuple{Vector{Float64}}, got a value of type Tuple{Vector{Float64}}
Stacktrace:
 [1] hcat
   @ ./array.jl:1668 [inlined]
 [2] strain_rect_fs()
   @ Main ./REPL[2]:2
 [3] stress_rect_fs()
   @ Main ./REPL[3]:2
 [4] top-level scope
   @ REPL[4]:1

or sometimes a segfault at

jl_is_submodule at ./src/module.c:819
operator() at ./src/codegen.cpp:6261 [inlined]
emit_function at ./src/codegen.cpp:6301
jl_emit_code at ./src/codegen.cpp:7077
jl_emit_codeinst at ./src/codegen.cpp:7111
jl_compile_workqueue at ./src/codegen.cpp:7212
_jl_compile_codeinst at ./src/jitlayers.cpp:105
jl_generate_fptr at ./src/jitlayers.cpp:313
jl_compile_method_internal at ./src/gf.c:1923
jl_compile_method_internal at ./src/gf.c:1877 [inlined]
_jl_invoke at ./src/gf.c:2183 [inlined]
jl_apply_generic at ./src/gf.c:2373
jl_apply at ./src/julia.h:1687 [inlined]
do_call at ./src/interpreter.c:115
eval_value at ./src/interpreter.c:204
eval_stmt_value at ./src/interpreter.c:155 [inlined]
eval_body at ./src/interpreter.c:557
jl_interpret_toplevel_thunk at ./src/interpreter.c:669
[...]

Seems to be directly related to the complexity of the method, with all the gigantic broadcasts inlined into it. Removing one of the RDSetupS() calls (which never get actually made) makes the problem go away. But to get an idea of the sheer size of the thing being compiled here:

julia> @code_typed strain_rect_fs()
[...]
46318 ─ %158646 = %new(Base.Broadcast.Broadcasted{Nothing, Tuple{Base.OneTo{Int64}}, typeof(-), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(+), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(-), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(+), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(/), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(*), Tuple{Float64, Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}}}, Int64}}, Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(/), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(*), Tuple{Float64, Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}}}, Int64}}}}, Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(*), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{0}, Nothing, typeof(/), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{0}, Nothing, typeof(/), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{0}, Nothing, typeof(/), Tuple{Float64, Int64}}, Irrational{:π}}}, Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{0}, Nothing, typeof(-), Tuple{Int64, Float64}}}}, Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(+), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(/), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(*), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(-), Tuple{Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}}}, Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}}}, Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}}}, Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(/), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(*), Tuple{Float64, Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}, Float64}}, Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}}}, Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(/), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(*), Tuple{Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}, Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}, Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}}}, Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}}}, Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(/), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(*), Tuple{Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}, Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}, Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}}}, Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}}}}}}}}}, Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(*), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{0}, Nothing, typeof(/), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{0}, Nothing, typeof(/), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{0}, Nothing, typeof(/), Tuple{Float64, Int64}}, Irrational{:π}}}, Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{0}, Nothing, typeof(-), Tuple{Int64, Float64}}}}, Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(+), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(/), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(-), Tuple{Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}}}, Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}}}, Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(/), Tuple{Float64, Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}}}, Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(*), Tuple{Float64, Float64, Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}}}, Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(/), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(*), Tuple{Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}, Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}, Float64}}, Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}}}, Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(/), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(*), Tuple{Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}, Float64, Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}}}, Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}}}}}}}}}, Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(*), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{0}, Nothing, typeof(/), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{0}, Nothing, typeof(/), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{0}, Nothing, typeof(/), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{0}, Nothing, typeof(*), Tuple{Float64, Float64}}, Int64}}, Irrational{:π}}}, Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{0}, Nothing, typeof(-), Tuple{Int64, Float64}}}}, Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(+), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(*), Tuple{Float64, Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}}}, Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(/), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(*), Tuple{Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}, Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}}}, Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}}}, Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(/), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(*), Tuple{Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}, Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}}}, Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}}}}}}}}}, -, %158644, %158195)::Base.Broadcast.Broadcasted{Nothing, Tuple{Base.OneTo{Int64}}, typeof(-), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(+), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(-), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(+), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(/), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(*), Tuple{Float64, Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}}}, Int64}}, Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(/), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(*), Tuple{Float64, Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}}}, Int64}}}}, Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(*), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{0}, Nothing, typeof(/), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{0}, Nothing, typeof(/), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{0}, Nothing, typeof(/), Tuple{Float64, Int64}}, Irrational{:π}}}, Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{0}, Nothing, typeof(-), Tuple{Int64, Float64}}}}, Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(+), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(/), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(*), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(-), Tuple{Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}}}, Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}}}, Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}}}, Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(/), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(*), Tuple{Float64, Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}, Float64}}, Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}}}, Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(/), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(*), Tuple{Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}, Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}, Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}}}, Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}}}, Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(/), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(*), Tuple{Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}, Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}, Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}}}, Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}}}}}}}}}, Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(*), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{0}, Nothing, typeof(/), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{0}, Nothing, typeof(/), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{0}, Nothing, typeof(/), Tuple{Float64, Int64}}, Irrational{:π}}}, Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{0}, Nothing, typeof(-), Tuple{Int64, Float64}}}}, Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(+), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(/), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(-), Tuple{Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}}}, Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}}}, Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(/), Tuple{Float64, Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}}}, Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(*), Tuple{Float64, Float64, Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}}}, Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(/), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(*), Tuple{Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}, Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}, Float64}}, Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}}}, Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(/), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(*), Tuple{Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}, Float64, Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}}}, Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}}}}}}}}}, Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(*), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{0}, Nothing, typeof(/), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{0}, Nothing, typeof(/), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{0}, Nothing, typeof(/), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{0}, Nothing, typeof(*), Tuple{Float64, Float64}}, Int64}}, Irrational{:π}}}, Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{0}, Nothing, typeof(-), Tuple{Int64, Float64}}}}, Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(+), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(*), Tuple{Float64, Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}}}, Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(/), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(*), Tuple{Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}, Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}}}, Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}}}, Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(/), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(*), Tuple{Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}, Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}}}, Base.Broadcast.Extruded{Vector{Float64}, Tuple{Bool}, Tuple{Int64}}}}}}}}}}
[...]
46338%158999 = Base.Broadcast.axes(%158200)::Tuple{Base.OneTo{Int64}}%159000 = Base.Broadcast.axes(%158201)::Tuple{Base.OneTo{Int64}}
│                 Base.Broadcast.throwdm(%158999, %159000)::Union{}
└──────           unreachable
46339 ─           goto #46340
46340 ─           goto #46341
46341 ─           goto #46342
46342 ─           goto #46343
46343return %158200
46344return nothing
) => Union{Nothing, Vector{Float64}}

@maleadt
Copy link
Member

maleadt commented Sep 2, 2020

Seems to be directly related to the complexity of the method, with all the gigantic broadcasts inlined into it

Looks like it indeed:

julia: /home/tbesard/Julia/julia/src/ircode.c:334: void jl_encode_value_(jl_ircode_state *, jl_value_t *, int): Assertion `id <= UINT16_MAX' failed.
signal (6): Aborted
in expression starting at /home/tbesard/Julia/tools/creduce/main.jl.new:133
gsignal at /lib/x86_64-linux-gnu/libc.so.6 (unknown line)
abort at /lib/x86_64-linux-gnu/libc.so.6 (unknown line)
unknown function (ip: 0x7f2c1058540e)
__assert_fail at /lib/x86_64-linux-gnu/libc.so.6 (unknown line)
jl_encode_value_ at /home/tbesard/Julia/julia/src/ircode.c:334
jl_encode_value_ at /home/tbesard/Julia/julia/src/ircode.c:164
jl_encode_value_ at /home/tbesard/Julia/julia/src/ircode.c:295
jl_compress_ir at /home/tbesard/Julia/julia/src/ircode.c:729
maybe_compress_codeinfo at ./compiler/typeinfer.jl:130
transform_result_for_cache at ./compiler/typeinfer.jl:148 [inlined]
cache_result! at ./compiler/typeinfer.jl:167 [inlined]
typeinf at ./compiler/typeinfer.jl:64
(gdb) call jl_(v)
SSAValue(158850)
(gdb) p id
$13 = 65536
(gdb) p ((jl_array_t*)s->method->roots)->length
$11 = 65537

That's quite some roots...

@martinholters
Copy link
Member

Ah, good ol' assertions, if only I remembered to enable them routinely...

So I guess we should either replace the assertion with a (conditional) throw ("Sorry, method too complicated, try println("Hello world!") instead"), or support going to 32bit ids here?

@maleadt
Copy link
Member

maleadt commented Sep 2, 2020

I guess the latter is easy enough:

diff --git a/src/ircode.c b/src/ircode.c
index 300b300c08..9b37273ff8 100644
--- a/src/ircode.c
+++ b/src/ircode.c
@@ -333,2 +333 @@ static void jl_encode_value_(jl_ircode_state *s, jl_value_t *v, int as_literal)
-            else {
-                assert(id <= UINT16_MAX);
+            else if (id < UINT16_MAX) {
@@ -336,0 +336,4 @@ static void jl_encode_value_(jl_ircode_state *s, jl_value_t *v, int as_literal)
+            } else {
+                assert(id <= UINT32_MAX);
+                write_uint8(s->s, TAG_EXTREME_METHODROOT);
+                write_uint32(s->s, id);
@@ -583,0 +587,2 @@ static jl_value_t *jl_decode_value(jl_ircode_state *s) JL_GC_DISABLED
+    case TAG_EXTREME_METHODROOT:
+        return jl_array_ptr_ref(s->method->roots, read_uint32(s->s));
diff --git a/src/serialize.h b/src/serialize.h
index 07ff46fdf9..c3555dd329 100644
--- a/src/serialize.h
+++ b/src/serialize.h
@@ -64,0 +65 @@ extern "C" {
+#define TAG_EXTREME_METHODROOT 57
@@ -110,0 +112,12 @@ static uint16_t read_uint16(ios_t *s) JL_NOTSAFEPOINT
+static void write_uint32(ios_t *s, uint32_t i) JL_NOTSAFEPOINT
+{
+    ios_write(s, (char*)&i, 4);
+}
+
+static uint32_t read_uint32(ios_t *s) JL_NOTSAFEPOINT
+{
+    uint32_t x = 0;
+    ios_read(s, (char*)&x, 4);
+    return x;
+}

I'll verify and make a PR.

@maleadt
Copy link
Member

maleadt commented Sep 2, 2020

Hmm, adding support for very long lists of method roots works, but it's not a great solution because the reproducer now just runs indefinitely. The huge amount of code is probably the real issue; I'll try to bisect that.

@martinholters
Copy link
Member

FWIW, I've tried basically the same patch (but using int32 instead of uint32) and the reproducer eventually ran to completion.

Furthermore, on 1.5.1, @code_typed strain_rect_fs() isn't exactly shorter, ending in:

│       %159712 = Base.ifelse(%159711, 0, %159710)::Int64
│       %159713 = %new(Base.OneTo{Int64}, %159712)::Base.OneTo{Int64}
│       %159714 = Core.tuple(%159713)::Tuple{Base.OneTo{Int64}}
│                 invoke Base.Broadcast.throwdm(%159714::Tuple{Base.OneTo{Int64}}, %159035::Tuple{Base.OneTo{Int64}})::Union{}
└──────           $(Expr(:unreachable))::Union{}
47679 ┄           goto #47680
47680 ─           goto #47681
47681 ─           goto #47682
47682 ─           goto #47683
47683 ─           return %159040
47684 ─           return
) => Union{Nothing, Array{Float64,1}}

So is the regression just in the number of roots produced?

@maleadt
Copy link
Member

maleadt commented Sep 2, 2020

Run-time on 1.4 seems a lot lower though.

@maleadt
Copy link
Member

maleadt commented Sep 3, 2020

So while #37370 fixes the immediate issue, I noticed how compile time of this reproducer has significantly regressed (+150%) since Julia 1.4. Bisecting that points to #35260:

cc60547e7f046272a5dcfdc013a50e70f47657e8 is the first bad commit
commit cc60547e7f046272a5dcfdc013a50e70f47657e8
Author: Christopher Rackauckas <accounts@chrisrackauckas.com>
Date:   Wed Mar 25 19:25:27 2020 -0400

    Inline and use a non-splatting combine_axes in broadcast (#35260)
    
    * Inline and use a non-splatting combine_axes in broadcast
    * test to ensure no allocations

:040000 040000 3b0df593bf45734bd4df095d83875fa462162444 e4b16e846755583f5a44ab54df2ec93e6421daac M      base
:040000 040000 2ee81a1111c74bfadd36cd36bdc89f7aefceb930 f5a58c58afcf9ca2b7d709db1a743aedb4be0152 M      test

On cc60547:

./julia ../bisect.jl  158.44s user 1.15s system 100% cpu 2:39.09 total

julia> @code_typed strain_rect_fs()
...
35677 ─           return %112520
35678 ─           return
) => Union{Nothing, Array{Float64,1}}

On the commit before that:

./julia ../bisect.jl  61.03s user 0.89s system 100% cpu 1:01.36 total

julia> @code_typed strain_rect_fs()
...
16483 ─          return %45436
16484 ─          return
) => Union{Nothing, Array}

What's the importance of that improvement / relevance of the huge sequence of broadcasts here? @ChrisRackauckas @mbauman

@ChrisRackauckas
Copy link
Member

ChrisRackauckas commented Sep 3, 2020

Without inlining that lots of standard broadcast expressions allocate and are about 50% slower. See https://discourse.julialang.org/t/strange-allocations-during-broadcasting/26605/10 for example (DiffEqBase.jl carried around a patch for awhile that inlined this since 1.0 for this reason). If standard broadcast expressions are slower than loops then they are no longer just a generic option but instead a semantic simplification / GPU support with a cost vs a loop, so I think it's important to have that inlining to prevent those allocations. This is the last piece required to make them equivalent.

On @martinholters 's example it probably makes it a whole lot faster than that too, though indeed standard broadcast has some issues with huge expressions. (Though I don't understand that reproducer since half of those expressions look like they don't return anything and would be deleted?)

@martinholters
Copy link
Member

That reproducer was obtained by reducing the original one, and as the return values were not required to reproduce the issue, gone they were.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or unintended behavior regression Regression in behavior compared to a previous version
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants