Skip to content

Commit

Permalink
Don't specialize groupreduce! on result array element type
Browse files Browse the repository at this point in the history
Works around crash seen in #2326. The inferred return type of
`groupreduce_init` is `Union{Vector{Any}, SentinelVector{Float64}}` and
it seems the compiler then crashes when trying to correctly identify `U`
from that union of types. Part of my conclusion here is based on the
fact that if you remove all other argument type constraints and just
make `groupreduce!` return `res` directly, it still crashes; thus, by
deduction, the crash has something to do with the compiler having
trouble with the `::AbstractVector{U}` argument type
constraint/specialization.

The work-around is pretty uncontroversial; we were already calling
`eltype(res)` in several other places, and I've checked that it infers
the same. I didn't add a test since this seems like such an obscure
compiler bug that it doesn't really seem necessary for DataFrames to be
testing core compiler behavior.

Also note that this bug exists in Julia <= 1.5, so current Julia master
(pending 1.6), which includes a number of compiler refactorings/changes,
seems to have resolved whatever the issue was.

cc: @Keno, @vtjnash, @JeffBezanson
  • Loading branch information
quinnj committed Jul 27, 2020
1 parent 86b9a4f commit a12a6cd
Showing 1 changed file with 4 additions and 4 deletions.
8 changes: 4 additions & 4 deletions src/groupeddataframe/splitapplycombine.jl
Original file line number Diff line number Diff line change
Expand Up @@ -962,8 +962,8 @@ function copyto_widen!(res::AbstractVector{T}, x::AbstractVector) where T
return res
end

function groupreduce!(res::AbstractVector{U}, f, op, condf, adjust, checkempty::Bool,
incol::AbstractVector{T}, gd::GroupedDataFrame) where {U,T}
function groupreduce!(res, f, op, condf, adjust, checkempty::Bool,
incol::AbstractVector{T}, gd::GroupedDataFrame) where {T}
n = length(gd)
if adjust !== nothing || checkempty
counts = zeros(Int, n)
Expand All @@ -974,7 +974,7 @@ function groupreduce!(res::AbstractVector{U}, f, op, condf, adjust, checkempty::
x = incol[i]
if gix > 0 && (condf === nothing || condf(x))
# this check should be optimized out if U is not Any
if U === Any && !isassigned(res, gix)
if eltype(res) === Any && !isassigned(res, gix)
res[gix] = f(x, gix)
else
res[gix] = op(res[gix], f(x, gix))
Expand All @@ -985,7 +985,7 @@ function groupreduce!(res::AbstractVector{U}, f, op, condf, adjust, checkempty::
end
end
# handle the case of an unitialized reduction
if U === Any
if eltype(res) === Any
if op === Base.add_sum
initf = zero
elseif op === Base.mul_prod
Expand Down

0 comments on commit a12a6cd

Please sign in to comment.