Fix gaussian integral #46
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #46 +/- ##
==========================================
+ Coverage 93.01% 93.97% +0.96%
==========================================
Files 18 18
Lines 1031 1046 +15
==========================================
+ Hits 959 983 +24
+ Misses 72 63 -9 ☔ View full report in Codecov by Sentry. |
|
Thank you for the fast implementation! Looks good to me :) |
| @@ -90,7 +90,19 @@ function fourier_filter(arr, fct=window_gaussian; kwargs...) | |||
| return fourier_filter!(copy(arr), fct; kwargs...) | |||
| end | |||
|
|
|||
| function fourier_filter_by_1D_FT!(arr::TA, wins::AbstractVector; transform_win=false, dims=(1:ndims(arr))) where {N, TA <: AbstractArray{<:Complex, N}} | |||
| """ | |||
| fourier_filter_by_1D_FT!(arr::TA, wins::AbstractVector; transform_win=false, normalize_win=false, dims=(1:ndims(arr))) where {N, TA <: AbstractArray{<:Complex, N}} | |||
There was a problem hiding this comment.
maybe transform_win and normalize_win.
I think being more verbose would be better here since win is confusing
There was a problem hiding this comment.
Sorry. Don't understand your comment. It is called "normalize_win" and you suggest the same?
We should definitely not change transform_win to stay backwards compatible. So I agree that "win" should be more verbose but consistency is also important.
There was a problem hiding this comment.
Ok didn't see this was already in the old version
| wins[d] | ||
| if (normalize_win) | ||
| if (wins[d][1] != 0 && wins[d][1] != 1) | ||
| wins[d] ./ wins[d][1] |
There was a problem hiding this comment.
this line is not covered by tests
| +`normalize_win`: specifies whether the directional windows (after potential FFT) each need to be normalized to a value of 1 at the zero frequency coordinate. | ||
| +`dims`: the dimensions to apply this separable multiplication to. If empty, the array will not be filtered. | ||
| """ | ||
| function fourier_filter_by_1D_RFT!(arr::TA, wins::AbstractVector; dims=(1:ndims(arr)), transform_win=false, normalize_win=false, kwargs...) where {T<:Real, N, TA<:AbstractArray{T, N}} |
fixed a bug with the normalization of Gaussian filtering. Added some tests, which also test the previously buggy case of zero-integral arrays.