BOUT-dev icon indicating copy to clipboard operation
BOUT-dev copied to clipboard

Stop using FFT by default in Delp2?

Open ZedThree opened this issue 4 years ago • 2 comments

Delp2 is a weird one. Its default implementation (the one with useFFT=true) isn't (I think anyway...) what you'd expect unless you'd read the source code to know it's designed to be (part of) the inverse of the FFT-based Laplace inversions... I'm most likely to use it for diffusion operators where the relation to Laplace inversion isn't important, and I imagine that's common. A nice solution might be to change the default value of useFFT to false for v5 (while we're allowed to break things), and then throw an exception if you do ask for useFFT=true with 3d metrics.

I also can't find Delp2's odd behaviour documented in the manual, which seems like a somewhat important omission...

Originally posted by @johnomotani in https://github.com/boutproject/BOUT-dev/pull/2025#discussion_r554101758

ZedThree avatar Jan 08 '21 18:01 ZedThree

The fft implementation is slightly odd. I think in some ways it ways it would be good to group it together with the Laplacian inversion, since it's supposed to be the inverse operation.

For cases where it matters that the forward and backward operators are identical, Delp2 and Laplace are only inverses for particular solvers (eg cyclic, same settings). The Petsc solver could quite easily supply a different operator for applying the matrix rather than inverting it.

Could the Laplace base class have a forward() function, to go with the invert function it's currently got. The FFT version of Delp2 could go in there with the cyclic implementation This might also remove the need for code outside Laplace to access the tridiagonal coefficients. The Petsc implementation could have it's own version, or throw if we don't implement the forward operator.

The Delp2 function would then be more like the others rather than an anomaly

bendudson avatar Jan 08 '21 18:01 bendudson

Oh, I do like the idea of a forward method. Especially if we can also reduce the scope of the tridiagonal coefficients.

ZedThree avatar Jan 08 '21 18:01 ZedThree