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

Type instability in fftshift/ifftshift #36

Closed
ngedwin98 opened this issue Nov 13, 2019 · 0 comments · Fixed by #40
Closed

Type instability in fftshift/ifftshift #36

ngedwin98 opened this issue Nov 13, 2019 · 0 comments · Fixed by #40

Comments

@ngedwin98
Copy link

ngedwin98 commented Nov 13, 2019

Originally brought up in JuliaMath/FFTW.jl#99, but fftshift and ifftshift are reexports from this package.

It appears the problem is that circshift produces type instability when called with vector arguments for shifts (although it handles correctly):

julia> z = randn(100)
julia> @code_warntype circshift(z, div.([size(z)...],2))
Body::Any
1 ─ %1  = (Base.arraysize)(a, 1)::Int64
│   %2  = $(Expr(:foreigncall, :(:jl_alloc_array_1d), Array{Float64,1}, svec(Any, Int64), :(:ccall), 2, Array{Float64,1}, :(%1), :(%1)))::Array{Float64,1}
│   %3  = (Core._apply)(Core.tuple, shiftamt)::Tuple{Vararg{Int64,N} where N}
│   %4  = (Base.map)(Base.Integer, %3)::Any
│   %5  = (isa)(%4, Tuple{})::Bool
└──       goto #3 if not %5
2 ─ %7  = (Base.arraylen)(a)::Int64
│   %8  = invoke Base.copyto!(%2::Array{Float64,1}, 1::Int64, _2::Array{Float64,1}, 1::Int64, %7::Int64)::Array{Float64,1}
└──       goto #4
3 ─ %10 = (Base.circshift!)(%2, a, %4)::Any
└──       goto #4
4 ┄ %12 = φ (#2 => %8, #3 => %10)::Any
└──       return %12

whereas this problem doesn't appear when called with a tuple for shifts:

julia> @code_warntype circshift(z, div.(size(z),2))
Body::Array{Float64,1}
1 ─ %1 = (Base.arraysize)(a, 1)::Int64
│   %2 = $(Expr(:foreigncall, :(:jl_alloc_array_1d), Array{Float64,1}, svec(Any, Int64), :(:ccall), 2, Array{Float64,1}, :(%1), :(%1)))::Array{Float64,1}
│   %3 = invoke Base.circshift!(%2::Array{Float64,1}, _2::Array{Float64,1}, _3::Tuple{Int64})::Array{Float64,1}
└──      return %3

It seems like the solution is to change all the shifts in fftshift and ifftshift to tuples?

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

Successfully merging a pull request may close this issue.

1 participant