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

Base.any has different semantics from base implementation #154

Open
ssz66666 opened this issue Aug 28, 2018 · 1 comment
Open

Base.any has different semantics from base implementation #154

ssz66666 opened this issue Aug 28, 2018 · 1 comment

Comments

@ssz66666
Copy link
Contributor

ssz66666 commented Aug 28, 2018

The Base.mapreduce based implementation of Base.any and Base.all

Base.any(A::GPUArray{Bool}) = mapreduce(identity, |, A; init = false)

currently has slightly different semantics from the implementation in Base, in two ways:

  1. The base implementation specifies that any and all are short-circuiting reductions. Their semantics can differ if the predicate applied is side-effecting (note any(p, itr) which is implemented in Support Transpose and Adjoint in broadcast better #148 ). Would retaining the difference and clarifying it in documentation be reasonable?

  2. The base implementation implements three-valued logic if the input iterable contains missing values. We might well implement it with something like:

function any(p, itr::GPUArray)
    v, m = mapreduce(
        x -> begin
            b = p(x)
            ismissing(b) ? (false, true) : (b::Bool, false)
        end,
        ((v1, m1), (v2, m2)) ->
            (v1 || v2, m1 || m2),
        itr,
        init = (false, false)
    )
    v || (m ? missing : false)
end

function all(p, itr::GPUArray)
    v, m = mapreduce(
        x -> begin
            b = p(x)
            ismissing(b) ? (true, true) : (b::Bool, false)
        end,
        ((v1, m1), (v2, m2)) ->
            (v1 && v2, m1 || m2),
        itr,
        init = (true, false)
    )
    v && (m ? missing : true)
end

However, currently CuArrays doesn't handle missing values correctly yet (~~~I'll write an issue there as well~~~ see issue here). I don't know if it makes sense to implement this in GPUArrays and make CuArrays fail the test.

@maleadt maleadt added the bug label Feb 12, 2019
@maleadt
Copy link
Member

maleadt commented Feb 24, 2020

Revisiting this, I don't think we can provide the short-circuiting behavior without completely sacrificing performance. Documenting the difference would seem like a good thing to do.

@maleadt maleadt added documentation and removed bug labels Feb 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants