-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
Dimensional any
and all
reduce with Boolean operations
#58185
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
Conversation
9f04f17
to
fd531e4
Compare
I split this out from #55628 as it seems slightly more clearly like a bugfix — but do note that it required some changes to doctests! Specifically, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
preserve and test more of the existing behaviors
@testset "issue #45562" begin | ||
@test all([true, true, true], dims = 1) == [true] | ||
@test any([true, true, true], dims = 1) == [true] | ||
@test_throws TypeError all([3, 3, 3], dims = 1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this line the only intended behavior change of this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, along with the corresponding change to any(; dims)
and any!
and all!
.
and whitespace in tests Co-authored-by: Lilith Orion Hafner <lilithhafner@gmail.com>
Seems like a good idea to require the arguments to Does this impact performance? |
Just to detangle all the reductions work here:
|
Yeah, looks like this inhibits SIMD. On this branch right now: julia> @btime any(x; dims=1) setup=(x=falses(10000,10))
126.083 μs (3 allocations: 112 bytes)
1×10 BitMatrix:
0 0 0 0 0 0 0 0 0 0
julia> @eval Base.or_any(x,y) = Bool(Bool(x) | Bool(y))
julia> @btime any(x; dims=1) setup=(x=falses(10000,10))
37.084 μs (3 allocations: 112 bytes)
1×10 BitMatrix:
0 0 0 0 0 0 0 0 0 0
julia> @eval Base.or_any(x::Bool,y::Bool) = (x | y)::Bool
julia> @btime any(x; dims=1) setup=(x=falses(10000,10))
27.458 μs (3 allocations: 112 bytes)
1×10 BitMatrix:
0 0 0 0 0 0 0 0 0 0 The last matches nightly. |
and more tests
The package evaluation job you requested has completed - possible new issues were detected. Report summary❗ Packages that crashed2 packages crashed on the previous version too. ✖ Packages that failed29 packages failed only on the current version.
1418 packages failed on the previous version too. ✔ Packages that passed tests24 packages passed tests only on the current version.
5259 packages passed tests on the previous version too. ~ Packages that at least loaded1 packages successfully loaded only on the current version.
2855 packages successfully loaded on the previous version too. ➖ Packages that were skipped altogether916 packages were skipped on the previous version too. |
Great, I think this is looking good. All the failures look to be numeric instabilities or timeouts in packages that tend to have numeric instabilities or timeouts... and this should manifest as a very clear new |
…#58185) and not bitwise ones. Specifically, they are now reductions using these operators: ```julia and_all(x, y) = (x && y)::Bool or_any(x, y) = (x || y)::Bool # As a performance optimization, avoid runtime branches: and_all(x::Bool, y::Bool) = (x & y)::Bool or_any(x::Bool, y::Bool) = (x | y)::Bool ``` Fixes half of JuliaLang#45562. --------- Co-authored-by: Lilith Hafner <Lilith.Hafner@gmail.com> Co-authored-by: Lilith Orion Hafner <lilithhafner@gmail.com>
and not bitwise ones. Specifically, they are now reductions using these operators:
Fixes half of #45562.