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

Reductions of Transpose, Adjoint, PermutedDimsArray #39513

Merged
merged 11 commits into from
Apr 19, 2021

Conversation

mcabbott
Copy link
Contributor

@mcabbott mcabbott commented Feb 4, 2021

Closes #33029.

It also adds some methods to such that adjoint ∘ identity ∘ adjoint === identity, since it will tend to produce such combinations, and something else may wish to dispatch on them. But it would work without this, I can separate it if that's preferable. (Perhaps we should also complete the group with conj ∘ transpose = adjoint etc.) Now removed, in favour of an explicit _sandwich function.

The reduction functions which this overloads for PermutedDimsArray were defined after its file is loaded. I moved this file later, but could equally well move these methods to another file.

@kshyatt kshyatt added the arrays [a, r, r, a, y, s] label Feb 6, 2021
@mcabbott
Copy link
Contributor Author

Bump. Perhaps paging @tkf, since you were involved in #33029

base/permuteddimsarray.jl Outdated Show resolved Hide resolved
base/permuteddimsarray.jl Outdated Show resolved Hide resolved
@vtjnash vtjnash added the forget me not PRs that one wants to make sure aren't forgotten label Apr 16, 2021
Co-authored-by: Jameson Nash <vtjnash@gmail.com>
@vtjnash vtjnash added merge me PR is reviewed. Merge when all tests are passing and removed forget me not PRs that one wants to make sure aren't forgotten labels Apr 16, 2021
@vtjnash vtjnash merged commit 58bde18 into JuliaLang:master Apr 19, 2021
@simeonschaub simeonschaub removed the merge me PR is reviewed. Merge when all tests are passing label Apr 28, 2021
ElOceanografo pushed a commit to ElOceanografo/julia that referenced this pull request May 4, 2021
antoine-levitt pushed a commit to antoine-levitt/julia that referenced this pull request May 9, 2021
@mcabbott mcabbott deleted the adjointsum branch May 13, 2021 15:49
johanmon pushed a commit to johanmon/julia that referenced this pull request Jul 5, 2021
@mcabbott
Copy link
Contributor Author

This doesn't seem speed up maximum:

julia> X = randn(5000, 200);

julia> @btime maximum($X);
  636.708 μs (0 allocations: 0 bytes)

julia> @btime maximum(transpose($X));
  4.643 ms (0 allocations: 0 bytes)

julia> @btime maximum($X; dims=1);
  744.458 μs (4 allocations: 1.88 KiB)

julia> @btime maximum(transpose($X); dims=2);
  4.644 ms (14 allocations: 2.06 KiB)

although sum works as expected:

julia> @btime sum($X);
  200.250 μs (0 allocations: 0 bytes)

julia> @btime sum(transpose($X));
  200.250 μs (0 allocations: 0 bytes)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrays [a, r, r, a, y, s]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reductions of Transpose, Adjoint, PermutedDimsArray
4 participants