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

Clean up internally Standard, Lagged Convolution, and Splined DLF for Hankel and Fourier transform. #10

Closed
prisae opened this issue Apr 26, 2018 · 1 comment
Labels
enhancement New feature or request
Milestone

Comments

@prisae
Copy link
Member

prisae commented Apr 26, 2018

A recent conversation with @sgkang from SimPEG reminded me of some chaotic things within empymod, whose roots lie in the way the code grew over time, and also how I learned over time. For instance, the original DLF functions are called fht (Fast Hankel Transform), as only the Hankel transform was considered, time-domain got added later. And over time different possibilities were added for Hankel DLF and Fourier DLF, the most recent is Standard Fourier DLF which was only added with empymod v1.5.2.

DLF implementation and their different optimisations

The methods that are possible with empymod for the Hankel and Fourier DLF are:

  • Standard DLF (No optimisation)
  • Lagged Convolution DLF (interpolation in output domain); spacing defined by filter
  • Splined DLF (interpolation in input domain), spacing defined by pts_per_dec.

Status quo

At the moment, this is differently implemented for Hankel and for Fourier:

Hankel DLF

  • standard: pts_per_dec=None
  • lagged: pts_per_dec=None, opt='spline'
  • splined: pts_per_dec>0, opt='spline'

Fourier DLF

  • standard: pts_per_dec<1
  • lagged: pts_per_dec=None
  • splined: pts_per_dec>0

Idea forward

I think a good implementation would be to have for both, Hankel and Fourier:

  • standard: pts_per_dec=0
  • lagged: pts_per_dec<0
  • splined: pts_per_dec>0

Therefore only having one parameter, and getting rid of opt='spline'.

This would mainly affect empymod.transform, and implementation is relatively easy. Functionality in empymod.utils could be extended to keep it backwards-compatible for all empymod.model-routines (including opt='spline'). However, the changes would be backwards incompatible for empymod.transform-functions. So it has to be done carefully to not break code which depend on these routines.

@prisae prisae added the enhancement New feature or request label Apr 26, 2018
@sgkang
Copy link

sgkang commented Apr 27, 2018

Thanks a lot @prisae . I am looking forward to playing with this option in https://github.com/simpeg/simpegEM1D

@prisae prisae added this to the v1.6.0 milestone Apr 29, 2018
@prisae prisae closed this as completed in 9bed72b Apr 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants