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

RFC: Add similar for PermutedDimsArray #23263

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

Keno
Copy link
Member

@Keno Keno commented Aug 14, 2017

This allows you to do things like:

julia> A = [1 2 3;4 5 6]
2×3 Array{Int64,2}:
 1  2  3
 4  5  6

julia> X = PermutedDimsArray(A, [2,1])
3×2 PermutedDimsArray{Int64,2,(2, 1),(2, 1),Array{Int64,2}}:
 1  4
 2  5
 3  6

julia> Y = Base.collect_similar(X, (x for x in Any[1 2.0; 3 4; 4 6]))
3×2 PermutedDimsArray{Real,2,(2, 1),(2, 1),Array{Real,2}}:
 1  2.0
 3  4
 4  6

julia> Y.parent
2×3 Array{Real,2}:
 1    3  4
 2.0  4  6

In particular I wanted for collecting into an array from an iterator that yielded its elements in transposed order, but it seems like a sensible definition regardless.

@ararslan ararslan added the domain:arrays [a, r, r, a, y, s] label Aug 15, 2017
@ararslan ararslan requested a review from timholy August 15, 2017 01:24
Copy link
Sponsor Member

@timholy timholy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea

PermutedDimsArray{S,N,perm,iperm,typeof(new_parent)}(new_parent)
end

function Base.similar(a::PermutedDimsArray{T,N,perm,iperm}, ::Type{S}, dims::Dims{N}) where {T,S,N,perm,iperm}
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need to define any more than this one definition? (Do fallbacks not handle the rest?)

end

function Base.similar(a::PermutedDimsArray{T,N,perm,iperm}, ::Type{S}, dims::Dims{N}) where {T,S,N,perm,iperm}
new_parent = similar(a.parent, S, ntuple(i->dims[perm[i]], N))
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this inferrable? (Do we need ntuple(f, Val{N})?)

@andyferris
Copy link
Member

Hmm... here was about to use PermutedDimsArray to do lazy transpose matrices and not have this behavior... :)

Is there a convenient way to get a "similar" array that strips the PermutedDims wrapper?

@timholy
Copy link
Sponsor Member

timholy commented Aug 15, 2017

Is there a convenient way to get a "similar" array that strips the PermutedDims wrapper?

Does similar(Array{eltype(A)}, indices(A)) do what you want? (You can specialize this syntax, of course.)

@andyferris
Copy link
Member

Well, it would work when you want Array... I'm looking for similar of the parent.

What I really want (and will implement separately to PermutedDimsArray if required) is similar(m::TransposedMatrix, T::Type, d::Dims) = similar(parent(m), T, d). I was just about to hook into const TransposedMatrix = PermutedDimsArray{(2,1)} (shortened form).

None of this means this PR is bad. But when I implement similar on my own generic wrapper types I tend to ditch the wrapper (since they tend to add a layer of inefficiency) and use the parent to control similar. The minimal PR for that here is:

function Base.similar(a::PermutedDimsArray, ::Type{T}, dims::Dims) where {T}
    similar(a.parent, T, dims)
end

@andreasnoack
Copy link
Member

Maybe time to start implementing the storagetype function that we have been discussing the PRs to remove full?

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

Successfully merging this pull request may close these issues.

5 participants