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

Allow type-unstable functions in mapslices #31217

Closed
wants to merge 1 commit into from
Closed

Conversation

ararslan
Copy link
Member

@ararslan ararslan commented Mar 1, 2019

Here's my attempt at fixing #26232, which doubles as an attempt to convince someone else to fix it better.

@ararslan ararslan added the arrays [a, r, r, a, y, s] label Mar 1, 2019
@ararslan ararslan requested a review from mbauman March 1, 2019 04:25
@nalimilan
Copy link
Member

I'm afraid the real fix is going to be more complex: we should use the same approach as map, i.e. reallocate the array if we encounter a slice with a new type.

@martinholters
Copy link
Member

Yep, we shouldn't use return_types to determine the element type of non-empty containers.

@mbauman
Copy link
Member

mbauman commented Mar 1, 2019

Oh I hate mapslices with a passion. Let's kill it.

In your use-case, is the inner function returning a scalar or an array? If it's a scalar, then use map(f, eachslice(A, dims=d)) — that'll "just work." If you need support for multiple dims, let's focus our efforts on adding that capability to eachslice.

If you're returning an array, though, map eachslice will return an array of arrays. You can then concatenate them together yourself, but there's definitely a performance advantage to fusing the two together. What if we were to internally use a datastructure like (view(R, :, i) for i in I) that would expose columns of a matrix as though they were single elements? And then used map!? That's not quite right, but I'd like to pursue something along these lines to pave the way towards a more consistent, reimagined version of this nonscalar application of mapslices.

@ararslan
Copy link
Member Author

ararslan commented Mar 1, 2019

See, my plan of getting someone else to fix it better is already working 😄

@ararslan
Copy link
Member Author

ararslan commented May 2, 2020

I will not be the one to fix it better. Hopefully someone else will. :)

@ararslan ararslan closed this May 2, 2020
@ararslan ararslan deleted the aa/mapslices branch May 2, 2020 20:04
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.

4 participants