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.stack of staticarrays creates non-static array #1272

Open
wsmoses opened this issue Aug 10, 2024 · 7 comments
Open

Base.stack of staticarrays creates non-static array #1272

wsmoses opened this issue Aug 10, 2024 · 7 comments

Comments

@wsmoses
Copy link

wsmoses commented Aug 10, 2024

x/ref EnzymeAD/Enzyme.jl#1714

@mcabbott
Copy link
Collaborator

mcabbott commented Sep 6, 2024

Not so clear what the input is on that Enzyme issue, but the output is a Matrix.

But xref JuliaLang/julia#52590, where the output is an MMatrix:

julia> stack([SA[1,2], SA[3,4], SA[5,6]])
2×3 Matrix{Int64}:
 1  3  5
 2  4  6

julia> stack(SA[SA[1,2], SA[3,4], SA[5,6]])
2×3 MMatrix{2, 3, Int64, 6} with indices SOneTo(2)×SOneTo(3):
 1  3  5
 2  4  6

@gdalle
Copy link

gdalle commented Oct 10, 2024

@mcabbott how would you go about handling the dims argument when creating SArray specializations for stack based on reduce(hcat/vcat)? When dims=1 or dims=2 I know what to do but other than that it seems less obvious.

@mcabbott
Copy link
Collaborator

mcabbott commented Oct 10, 2024

I'm not so sure what you mean. Can you specify what the input is?

All legal values of dims do already work, e.g. with the input above:

julia> stack(SA[SA[1,2], SA[3,4], SA[5,6]]; dims=1)
3×2 MMatrix{3, 2, Int64, 6} with indices SOneTo(3)×SOneTo(2):
 1  2
 3  4
 5  6

julia> stack(SA[SA[1,2], SA[3,4], SA[5,6]]; dims=2)
2×3 MMatrix{2, 3, Int64, 6} with indices SOneTo(2)×SOneTo(3):
 1  3  5
 2  4  6

julia> stack(SA[SA[1,2], SA[3,4], SA[5,6]]; dims=3)
ERROR: ArgumentError: cannot stack slices ndims(x) = 1 along dims = 3

To make this return an SArray not MArray, it may make sense to just use Base's code path & convert the object before it's returned? But note that in any case it cannot be type-stable, since MMatrix{3, 2, ...} != MMatrix{2, 3, ...}.

Edit: Without dims it can be type-stable. Likely any implementation should overload internal functions, like _stack(dims::Colon, xs::Static...)

@gdalle
Copy link

gdalle commented Oct 10, 2024

To make this return an SArray not MArray

Yeah that would be the goal

But note that in any case it cannot be type-stable

I hadn't thought of that... @wsmoses do you think it is better to have stack be type-stable and return an Array, or type-unstable and return an SArray?

@mateuszbaran
Copy link
Collaborator

I hadn't thought of that... @wsmoses do you think it is better to have stack be type-stable and return an Array, or type-unstable and return an SArray?

The default choice in StaticArrays.jl is to return Array when an operation can't be type-stable with SArray. If someone wanted to suggest the other way, it'd have to be supported by some benchmarks.

@gdalle
Copy link

gdalle commented Oct 14, 2024

Yeah I hadn't realized that stack carries dimensionality information in the values of the kwarg dims, so it has to be type-unstable. I think defaulting on Array is a reasonable choice then.

@mcabbott
Copy link
Collaborator

Note that dims is optional, and cases without it can be type-stable (when the input has enough static axes). At present these give MMatrix, as that's what falls out of Base calling similar.

Note also that many examples above with dims are at present type-unstable because they make an MMatrix, not an Array.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants