willow-ahrens / Finch.jl

Sparse tensors in Julia and more! Datastructure-driven array programing language.
http://willowahrens.io/Finch.jl/
MIT License
151 stars 12 forks source link

`floor_div` mixed dtypes #592

Closed mtsokol closed 3 weeks ago

mtsokol commented 3 weeks ago

Hi @willow-ahrens,

One of the array-api-tests failures is caused by returning abstract dtypes instead of concrete ones in floor_div for mixed data types. The lower one should be promoted to the larger size (rules):

julia> using Finch

julia> eltype(broadcast(Finch.fld_nothrow, Tensor(ones(Int16, 1)), Tensor(ones(Int8, 1))))
┌ Warning: Division by zero in fld
└ @ Finch ~/JuliaProjects/Finch.jl/src/util/special_functions.jl:125
Signed

julia> eltype(broadcast(Finch.fld_nothrow, Tensor(ones(Float64, 1)), Tensor(ones(Float32, 1))))
┌ Warning: Division by zero in fld
└ @ Finch ~/JuliaProjects/Finch.jl/src/util/special_functions.jl:125
┌ Warning: Division by zero in fld
└ @ Finch ~/JuliaProjects/Finch.jl/src/util/special_functions.jl:125
┌ Warning: Division by zero in fld
└ @ Finch ~/JuliaProjects/Finch.jl/src/util/special_functions.jl:125
AbstractFloat
willow-ahrens commented 3 weeks ago

oh, strange. You're right, this should return Float64. I'll look into it.

mtsokol commented 3 weeks ago

Thank you! For Int16 and Int8 it should return Int16 according to promotion rules.

willow-ahrens commented 3 weeks ago

ah, I see. The issue is actually in the definition.

"""
    fld_nothrow(x, y)

Returns `fld(x, y)` normally, returns zero and issues a warning if `y` is zero.
"""
fld_nothrow(x, y) = iszero(y) ? (@warn("Division by zero in fld"); zero(y)) : fld(x, y)

This definition returns Float32 or Float64 depending on whether y is zero

willow-ahrens commented 3 weeks ago

should be a quick fix.

willow-ahrens commented 3 weeks ago

I'll fix the floor-div functions, but also note:

The return type of Finch high-level interface is currently based on:

https://github.com/JuliaLang/julia/blob/46933b8a708453f9caecb8abce88770c4bff675d/base/promotion.jl#L508-L619

We may want to provide an interface inside Finch or otherwise which the user can directly modify to ensure predictable output types.