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

squeeze() does not infer number of dimensions in output array #4270

Closed
simonster opened this issue Sep 13, 2013 · 6 comments
Closed

squeeze() does not infer number of dimensions in output array #4270

simonster opened this issue Sep 13, 2013 · 6 comments

Comments

@simonster
Copy link
Member

e.g. code_typed(squeeze, (Matrix{Float64}, Int)) suggests that squeeze returns an Array{Float64,N}, but it seems like it should be possible to infer that it will be an Array{Float64,1}. This would be especially useful since squeeze is the natural way to deal with singleton dimensions returned by sum et al.

@JeffBezanson
Copy link
Sponsor Member

For this to be possible a bit of rewriting would be necessary. Currently some corner cases are possible, such as:

julia> squeeze(rand(2,2),(0,0,0))
2x2 Array{Float64,2}:
 0.154123  0.0170937
 0.296481  0.339155 

julia> squeeze(rand(2,2),(3,))
2x2 Array{Float64,2}:
 0.0312394  0.522369
 0.633781   0.698172

The first one is probably unintentional, but the second one arguably makes sense, unless we want to enforce that squeeze must be able to remove dimensions.

@simonster
Copy link
Member Author

Enforcing that squeeze must be able to remove dimensions seems fine to me. Are there scenarios where one would want to squeeze a dimension that may or may not exist? It doesn't seem particularly unsafe, but it doesn't seem all that useful either, at least for the code I write.

@pao
Copy link
Member

pao commented Sep 16, 2013

@simonster
Copy link
Member Author

I think that post is about a different aspect of squeeze. I agree with @StefanKarpinski that blindly dropping all singleton dimensions is a bad idea (and indeed, we definitely couldn't infer the number of dimensions in the output in that case). The question is whether squeeze should throw when the dimension to be squeezed exceeds the number of dimensions in the input (e.g. squeeze(Array{T,2}, 3)) so that squeeze(Array{T,N}, n) would always return an Array{T,N-1}.

@StefanKarpinski
Copy link
Sponsor Member

I think we should definitely fix squeeze so that the number of dimensions of the result can be statically inferred.

@pao
Copy link
Member

pao commented Sep 17, 2013

Wow, I totally missed that--sorry for the noise. False positive on my FAQ alarm.

simonster added a commit that referenced this issue Dec 8, 2014
This also makes `squeeze` require a tuple and deprecates the version
that takes an iterator. I doubt there are many instances where the
dimensions to be squeezed aren't given at runtime.
simonster added a commit that referenced this issue Dec 8, 2014
This also makes `squeeze` require a tuple and deprecates the version
that takes an iterator. I doubt there are many instances where the
dimensions to be squeezed aren't given at runtime.
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