Inconsistent return types between scalar and array operations with Bool arguments (and some other types)

[carlobaldassi]

These ones are related to broadcasting:

julia> true .+ true   # same for .-
2

julia> [true] .+ [true]
1-element Bool Array:
 true

julia> true .^ true
1

julia> [true] .^ [true]
1-element Bool Array:
 true

julia> true ./ 0x1   # same with 0x1 ./ 0x1
1.0f0

julia> [true] ./ [0x1]
1-element Float64 Array:
 1.0

julia> true .* true
1

julia> [true] .* [true]
1-element Bool Array:
 true

julia> true .* 0x1 # same with 0x1 .* 0x1
0x0000000000000001

julia> [true] .* [0x1]
1-element Uint8 Array:
 0x01

These are not related to broadcasting:

julia> div(true, true)  # same for mod
1

julia> div([true], [true])
1-element Bool Array:
 true

[JeffBezanson]

I almost think we should get rid of /, ^, div, and mod for Bool. They are pretty silly.

[johnmyleswhite]

+1

[carlobaldassi]

I agree some operations are silly when they just involve Bools, but they're not for a Bool and another type (e.g. true / 4), so it would look even sillier to just pick out just the Bool/Bool cases IMO.
Anyway, if we do remove those operations, there would still be the case of .+ for Bool, and of ./ for Uint8.

[StefanKarpinski]

Cases like true/4 can be handled via promotion and we can generally avoid special cases for them, I suspect.

[JeffBezanson]

I like that [true] .* [0x1] gives a Uint8 array; this is a masking operation, so it should give something of the same type as the non-Bool argument. The return type of the scalar case true .* 0x1 is not as important.

[carlobaldassi]

Cases like true/4 can be handled via promotion and we can generally avoid special cases for them, I suspect.

In fact, I was thinking more about the array case (e.g. you get an array of Bools, or a BitArray, via find, or .> or similar, and you want to apply ./ with some numeric matrix: there's no reason to go through a conversion for that). Also, I was not proposing to special-case it (it actually works fine, with the only exception of division by an Uint8 or a Uint16 I think), but instead to not-special-case (i.e. not remove) the Bool / Bool case, silly as it is (after all, it was decided that Bool is a numeric type, so one could expect to be able to do whatever valid arithmetic operation with it IMO, especially from a generic programming perspective).

I like that [true] .* [0x1] gives a Uint8 array; this is a masking operation, so it should give something of the same type as the non-Bool argument. The return type of the scalar case true .* 0x1 is not as important.

That seems reasonable. For .*, the only inconsistency would then be about [0x1] .* [0x1] and the corresponding scalar version.

[toivoh]

The methods in broadcast.jl for ./, .\, and .^ use functions in the same file called type_div and type_pow to pick a return type. These probably need to be elaborated. The methods for .+, .-, and .* do not use any such functions yet, (just promote_type) but that could of course be changed. So when we know what result types we want, please change them!

[carlobaldassi]

For .*, the only inconsistency would then be about [0x1] .* [0x1] and the corresponding scalar version.

Sorry, I hadn't realized this was already fixed. Of the original issues, only div and mod are still outstanding (while .* stays as-is), everything else is fixed. I guess I should now try to implement broadcasting for BitArrays then :)

div and mod also have more general issues than the ones reported (e.g. involving other types than Bool); I'll do some more systematic screening soon.

[carlobaldassi]

It seems I was wrong about all other issues being fixed... I wrote a systematic checker and the results (included in the gist) are kind of alarming (not a single function passes all tests, not even unary +). Not all of them are bugs of course, but I think having the code out and being able to run it to inspect the details and test changes would be useful.

The code also includes an additional check, to compare scalar div and mod return types of the same arguments, since for example:

julia> div(0x1, 1) | typeof
Uint64

julia> mod(0x1, 1) | typeof
Int64

This is actually a different issue (or maybe not an issue at all), but I thought it would fit in the test suite.

[JeffBezanson]

A lot of these have a reason --- for example the result of mod has the same sign as its second argument, so its signedness matches the second argument.

[StefanKarpinski]

Yes, the size and signedness of mod match its second argument, so the return type can safely be the same as the second argument and this tends to lead to more type-stable code. We should probably be more consistent about this across all of the mod methods. The case of rem is a little harder since the size matches the second argument but the signedness matches the first argument, so there are cases where the result can't fit into either argument type. We could do something tighter than promotion for rem though, such as:

rem{T<:Signed}(::Integer, ::T) :: T
rem{T<:Unsigned}(::Unsigned, ::T) :: T

The only problematic case is when the first argument is singed and the second argument is unsigned, i.e. rem{S<:Signed,T<:Unsigned}(::S, ::T). It's also somewhat arguable in that you could throw a domain error when the second argument is unsigned and the first argument is negative. I suspect that this situation is rare and that when it does occur mod is probably more correct than rem. That would allow rem to have the same type behavior as mod, which might be advantageous, although it seems a little fiddly given that we could give an answer with the correct value and sign. The argument that this situation is probably usually a bug is compelling though.

[stevengj]

As of #17623, I'm getting consistent types for boolean scalars and arrays:

julia> true .+ true 
2

julia> [true] .+ [true]
1-element Array{Int64,1}:
 2

julia> true .^ true
true

julia> [true] .^ [true]
1-element BitArray{1}:
 true

julia> true ./ 0x1
1.0

julia> [true] ./ [0x1]
1-element Array{Float64,1}:
 1.0

julia> true .* true
true

julia> [true] .* [true]
1-element Array{Bool,1}:
 true

julia> true .* 0x1
0x01

julia> [true] .* [0x1]
1-element Array{UInt8,1}:
 0x01

julia> div(true, true)
true

julia> div([true], [true])
1-element Array{Bool,1}:
 true

julia> div.([true], [true])
1-element Array{Bool,1}:
 true

[stevengj]

Closed by #17623.

[tkelman]

are all of these tested now?

[stevengj]

Probably not