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

Fixes bug in parsevalsum/parsevalsum2 #350

Merged
merged 4 commits into from
Feb 11, 2023
Merged

Fixes bug in parsevalsum/parsevalsum2 #350

merged 4 commits into from
Feb 11, 2023

Conversation

navidcy
Copy link
Member

@navidcy navidcy commented Feb 11, 2023

This PR fixes a bug in parsevalsum/parsevalsum2. When rfft transforms are provided, then the mode that corresponds to k = nx/2 should only be summed once (and not twice) since its conjugate is not part of rfft output.

The previous tests did not catch this because the functions we were giving as input for the parsevalsum tests were smooth and thus had no Fourier amplitude at k = nx/2.

when rfft is used, the mode k=nx/2 should only be summed once since its conjugate is not part of rfft output
@navidcy navidcy added the bug label Feb 11, 2023
@navidcy navidcy changed the title Clarification in parsevalsum2 docstring Fixes bug in parsevalsum/parsevalsum2 Feb 11, 2023
@glwagner
Copy link
Member

Maybe another reference?

@navidcy
Copy link
Member Author

navidcy commented Feb 11, 2023

What do you mean "another reference"?

@navidcy
Copy link
Member Author

navidcy commented Feb 11, 2023

julia> using FourierFlows
[ Info: FourierFlows will use 6 threads

julia> grid = OneDGrid(nx=6, Lx=2π)
OneDimensionalGrid
  ├─────────── Device: CPU
  ├──────── FloatType: Float64
  ├────────── size Lx: 6.283185307179586
  ├──── resolution nx: 6
  ├── grid spacing dx: 1.0471975511965976
  ├─────────── domain: x ∈ [-3.141592653589793, 2.094395102393195]
  └─ aliased fraction: 0.3333333333333333

julia> grid.kr
4-element Vector{Float64}:
 0.0
 1.0
 2.0
 3.0

julia> grid.k
6-element Vector{Float64}:
  0.0
  1.0
  2.0
 -3.0
 -2.0
 -1.0

The mode k = -nx/2 = -3 (in the example above) only appears once in the full wavenumber grid grid.k.
So when we only have the grid.kr wavenumber, we should not sum over kr = nx/2 twice.

@navidcy navidcy merged commit 5946ea9 into main Feb 11, 2023
@navidcy navidcy deleted the ncc/clarify branch February 11, 2023 05:40
@navidcy
Copy link
Member Author

navidcy commented Feb 11, 2023

I discovered the bug when I was testing out something and was doing

fh = randn(Complex{eltype(grid)}, (grid.nkr, grid.nl))
f = irfft(deepcopy(f), grid.nx)

and then tried to confirm that parsevalsum2(fh, grid) was the same as sum(f.^2) * grid.dx * grid.dy...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants