Bug in function all

I find a bug about all.

here is the result.

julia> all([3, 3, 3], dims = 1)
1-element Vector{Bool}:
 1

The result maybe wrong.

julia> for i in 1:10
           @show all(i * ones(Int, 3), dims = 1)
       end
all(i * ones(Int, 3), dims = 1) = Bool[1]
all(i * ones(Int, 3), dims = 1) = Bool[0]
all(i * ones(Int, 3), dims = 1) = Bool[1]
all(i * ones(Int, 3), dims = 1) = Bool[0]
all(i * ones(Int, 3), dims = 1) = Bool[1]
all(i * ones(Int, 3), dims = 1) = Bool[0]
all(i * ones(Int, 3), dims = 1) = Bool[1]
all(i * ones(Int, 3), dims = 1) = Bool[0]
all(i * ones(Int, 3), dims = 1) = Bool[1]
all(i * ones(Int, 3), dims = 1) = Bool[0]

We talk about this problem here

any throws an error:

julia> any([3, 3, 3], dims = 1)
ERROR: InexactError: Bool(3)
Stacktrace:
  [1] Bool
    @ ./float.jl:158 [inlined]
  [2] convert
    @ ./number.jl:7 [inlined]
  [3] setindex!
    @ ./array.jl:959 [inlined]
  [4] setindex!
    @ ./multidimensional.jl:674 [inlined]
  [5] _mapreducedim!(f::typeof(identity), op::typeof(|), R::Vector{Bool}, A::Vector{Int64})
    @ Base ./reducedim.jl:311
  [6] mapreducedim!(f::Function, op::Function, R::Vector{Bool}, A::Vector{Int64})
    @ Base ./reducedim.jl:324
  [7] _mapreduce_dim(f::Function, op::Function, #unused#::Base._InitialValue, A::Vector{Int64}, dims::Int64)
    @ Base ./reducedim.jl:371
  [8] #mapreduce#758
    @ ./reducedim.jl:357 [inlined]
  [9] #_any#812
    @ ./reducedim.jl:1023 [inlined]
 [10] _any
    @ ./reducedim.jl:1023 [inlined]
 [11] #_any#811
    @ ./reducedim.jl:1022 [inlined]
 [12] _any
    @ ./reducedim.jl:1022 [inlined]
 [13] #any#785
    @ ./reducedim.jl:1003 [inlined]
 [14] top-level scope
    @ REPL[428]:1

The error we want is ERROR: TypeError: non-boolean (Int64) used in boolean context

@pfitzseb

From @pfitzseb in the linked discussion

This boils down to

julia> mapreduce(identity, &, [3,3,3]; dims=1)
1-element Vector{Bool}:
1

julia> mapreduce(identity, &, [2,2,2]; dims=1)
1-element Vector{Bool}:
0

and definitely is a bug.

The problem is that we are using & and | for all and any. Unfortunately, || and && are not functions.

We should be using something that errors on non-booleans. We can write up custom functions and use these instead:

boolean_and(a::Bool, b::Bool) = a & b
boolean_and(a::Bool, b) = throw(TypeError(:boolean_and, Bool, b))
boolean_and(a, b) = throw(TypeError(:boolean_and, Bool, a))
boolean_or(a::Bool, b::Bool) = a | b
boolean_or(a::Bool, b) = throw(TypeError(:boolean_or, Bool, b))
boolean_or(a, b) = throw(TypeError(:boolean_or, Bool, a))

Notice that the investigation revealed two separate bugs with reductions over an axis.

This should error like its non-axis version:

julia> all([3, 3, 3], dims = 1)
1-element Vector{Bool}:
 1

julia> all([3, 3, 3])
ERROR: TypeError: non-boolean (Int64) used in boolean context

The downstream mapreduce also gives an incorrect result along an axis:

julia> mapreduce(identity, &, [3,3,3], dims = 1)
1-element Vector{Bool}:
 1

julia> mapreduce(identity, &, [3,3,3])
3

The reducedim part of this bug is caused by

julia/base/reducedim.jl

Lines 206 to 207 in bd8dbc3

    
           reducedim_init(f, op::typeof(&), A::AbstractArrayOrBroadcasted, region) = reducedim_initarray(A, region, true) 
        
           reducedim_init(f, op::typeof(|), A::AbstractArrayOrBroadcasted, region) = reducedim_initarray(A, region, false)

which causes the returned array to be unconditionally initialized as a Array{Bool}, even though &/| aren't guaranteed to return booleans.

@GunnarFarneback

@GunnarFarneback, the problem you labeled 2 is at the intersection of this issue and #45566.
The linked PR fixes 1 and turns 2 from a wrong answer into an error.

I assume you mean the opposite. It would be good if it added tests for both problems.

The expected result in 1 is an error, so by "fixes 1" I also mean "turn a wrong answer into an error", Another way I could have said it that might be more clear is "the linked PR turns both these cases into an error. However, 2 should not error. That should be fixed elsewhere".

I added tests for 1, the issue in the OP. Because I did't solve 2, I also didn't add tests for it because those tests would be broken.

Thanks, I misunderstood. Turning a wrong answer into an error sounds a bit scary, even if both are incorrect.

Unless I'm missing something, the middle method of each of these is unnecessary. You can just do:

boolean_and(a::Bool, b::Bool) = a & b
boolean_or(a::Bool, b::Bool) = a | b
boolean_and(a, b) = throw(TypeError(:boolean_and, Bool, a))
boolean_or(a, b) = throw(TypeError(:boolean_or, Bool, a))

Could generalize this to

function boolean_op(f::Function)
    op(a, b) = throw(TypeError(nameof(f), Bool, a))
    op(a::Bool, b::Bool) = f(a, b)
end

Although that does produce annoyingly ugly stack traces on account of the inner function.

This is unrelated to correctness, but shouldn't all/any short-circuit? There really is no need to check the whole array after all (although you'll of course end up with a sort of ragged iteration scheme which could be slower in some cases, I guess).

@pfitzseb

Unless I'm missing something, the middle method of each of these is unnecessary. You can just do:

I don't like this error:

julia> boolean_and(true, 5)
ERROR: TypeError: non-boolean (Bool) used in boolean context

Could generalize this

Yes, though I do err on the side of cleaner error messages.

shouldn't all/any short-circuit?

Thanks @pfitzseb! I expect tat this would better and obviate the whole issue. I'll look into it more.

This is unrelated to correctness, but shouldn't all/any short-circuit? There really is no need to check the whole array after all (although you'll of course end up with a sort of ragged iteration scheme which could be slower in some cases, I guess).

I agree with the short-circuting behavior being more efficient. But then the check on an array for example needs to be done separately, depending on how the short-circuiting is achieved. false && :hello would return false right away. So will need to check that an array is an Array{Bool} before, otherwise bugs could be hidden again...

We already have all([false, :hello]) == false. I think that this is okay because it is explicitly documented to be short circuited. It can also sometimes be somewhat challenging to determine whether an input is actually all Bools (e.g. all(function, itr) may require executing the function on every element of itr).

I think that it is appropriate for all(f, [a,b,c,d]) to be the same as f(a) && f(b) && f(c) && f(d).

Bump, would be good to get a fix in for this.

Dimensional `any` and `all` reduce with Boolean operations (#58185)

Dimensional `any` and `all` reduce with Boolean operations (JuliaLang…

This can be closed. (I don't have triage privileges)

julia> all([3, 3, 3]; dims=1)
ERROR: TypeError: non-boolean (Int64) used in boolean context
Stacktrace:
  [1] and_all
    @ ./reduce.jl:32 [inlined]

I think to fix

julia> mapreduce(identity, |, [1, 1, 1]; dims=1)
1-element Vector{Bool}:
 1

we still need #58418

Different issue, I think? all/any no longer go through that particular mapreduce path, so it solves the original issue, even if it didn't solve it by fixing mapreduce on |

ERROR: TypeError: non-boolean (Int64) used in boolean context
Stacktrace:
  [1] and_all
    @ ./reduce.jl:32 [inlined]
  [2] macro expansion
    @ ./reducedim.jl:281 [inlined]
  [3] macro expansion
    @ ./simdloop.jl:77 [inlined]
  [4] _mapreducedim!(f::typeof(identity), op::typeof(Base.and_all), R::Vector{Bool}, A::Vector{Int64})
    @ Base ./reducedim.jl:280
  [5] mapreducedim!
    @ ./reducedim.jl:297 [inlined]
  [6] _mapreduce_dim
    @ ./reducedim.jl:344 [inlined]
  [7] mapreduce
    @ ./reducedim.jl:330 [inlined]
  [8] _all
    @ ./reducedim.jl:1012 [inlined]
  [9] _all
    @ ./reducedim.jl:1011 [inlined]
 [10] kwcall(::@NamedTuple{dims::Int64}, ::typeof(all), a::Vector{Int64})
    @ Base ./reducedim.jl:995
 [11] top-level scope
    @ REPL[1]:1

i.e., I think this issue by itself could be closed.

Navigation Menu

Repository navigation

Bug in function `all` #45562

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	reducedim_init(f, op::typeof(&), A::AbstractArrayOrBroadcasted, region) = reducedim_initarray(A, region, true)
	reducedim_init(f, op::typeof(\|), A::AbstractArrayOrBroadcasted, region) = reducedim_initarray(A, region, false)

Bug in function all #45562

Description

Activity

zdlspace0528 commented on Jun 2, 2022

LilithHafner commented on Jun 2, 2022

LilithHafner commented on Jun 2, 2022

LilithHafner commented on Jun 2, 2022

LilithHafner commented on Jun 2, 2022

GunnarFarneback commented on Jun 3, 2022

pfitzseb commented on Jun 3, 2022

LilithHafner commented on Jun 3, 2022

GunnarFarneback commented on Jun 3, 2022

LilithHafner commented on Jun 3, 2022

GunnarFarneback commented on Jun 3, 2022

StefanKarpinski commented on Jun 3, 2022

pfitzseb commented on Jun 3, 2022

LilithHafner commented on Jun 3, 2022

raminammour commented on Jun 3, 2022

LilithHafner commented on Jun 3, 2022

LilithHafner commented on Jun 4, 2022

StefanKarpinski commented on May 5, 2023

MilesCranmer commented on Jan 4, 2026

adienes commented on Jan 4, 2026

MilesCranmer commented on Jan 4, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

Bug in function `all` #45562