FastTransforms.jl icon indicating copy to clipboard operation
FastTransforms.jl copied to clipboard

FIX(nufft): Impossible type restriction for nufft2

Open gwater opened this issue 5 years ago • 2 comments

It was impossible to run nufft2 because the input has to be real and complex at the same time.

gwater avatar Jan 28 '21 12:01 gwater

Codecov Report

Merging #131 (cb98aac) into master (8625a44) will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #131   +/-   ##
=======================================
  Coverage   78.41%   78.41%           
=======================================
  Files          13       13           
  Lines        1543     1543           
=======================================
  Hits         1210     1210           
  Misses        333      333           
Impacted Files Coverage Δ
src/nufft.jl 94.76% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 8625a44...cb98aac. Read the comment docs.

codecov[bot] avatar Jan 28 '21 12:01 codecov[bot]

Looks like a lot of the NUFFT type parameterizations are a bit ad hoc.

Some of the executions would benefit from another type parameter, such as https://github.com/JuliaApproximation/FastTransforms.jl/blob/8625a440488c79e40355ab966bb805dc9650cfde/src/nufft.jl#L84 and others would benefit from removing type parameters entirely https://github.com/JuliaApproximation/FastTransforms.jl/blob/8625a440488c79e40355ab966bb805dc9650cfde/src/nufft.jl#L99 Would you mind going through the files (nufft.jl and inufft.jl) to check cases in this PR?

MikaelSlevinsky avatar Jan 28 '21 16:01 MikaelSlevinsky