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

Compute upward derivative of a grid in the frequency domain #238

Merged
merged 58 commits into from
May 31, 2022

Conversation

santisoler
Copy link
Member

@santisoler santisoler commented May 31, 2021

Define a new derivative_upward function for computing the spatial upward
derivative of a 2D grid in the frequency domain. The function makes use of
xrft for handling Fourier transformations of xarray objects. Add a new
filters subpackage that includes FFT filters: functions that take grids in
frequency domain and return the desired filter also in frequency domain. Add
fft and ifft wrapper functions of the xrft.fft and xrft.ifft ones. Add
a new apply_filter function that takes a grid in the spatial domain, applies
fft, the filter and ifft and returns the filtered grid also in spatial domain.
Add tests for the new features and a gallery example for the upward derivative.
Add netcdf4 as requirement for testing.

Reminders:

  • Run make format and make check to make sure the code follows the style guide.
  • Add tests for new features or tests that would have caught the bug that you're fixing.
  • Add new public functions/methods/classes to doc/api/index.rst and the base __init__.py file for the package.
  • Write detailed docstrings for all functions/classes/methods. It often helps to design better code if you write the docstrings first.
  • If adding new functionality, add an example to the docstring, gallery, and/or tutorials.
  • Add your full name, affiliation, and ORCID (optional) to the AUTHORS.md file (if you haven't already) in case you'd like to be listed as an author on the Zenodo archive of the next release.

@santisoler santisoler changed the title Compute grid derivatives in the frequency domain WIP Compute grid derivatives in the frequency domain May 31, 2021
@RichardScottOZ
Copy link
Contributor

Seems good - is 'vertical' terminology more used than upward?

Notes for the function could I suppose include parenthetical references to x/y after easting and northing?

@santisoler
Copy link
Member Author

Thanks @RichardScottOZ for the feedback!

Seems good - is 'vertical' terminology more used than upward?

"Vertical" is more widely used, although the drawback of using it is that it doesn't explicit specify the direction of the "vertical" axe. That's why in Harmonica we decided to use "upward" for specifying the non-horizontal direction explicitly stating that it points up.

Notes for the function could I suppose include parenthetical references to x/y after easting and northing?

And a similar thing happens with x and y. We decided to use "easting" and "northing" for the horizontal axes, this way we avoid the need to constantly remember where are the "x" and "y" axes pointing (some software use "x" for northing, while some other uses for the easting).

Do you feel that using easting, northing and upward might be confusing?

@ThomasMGeo
Copy link

I would just put in the comments that upward is also called vertical, but I do like your reasoning of calling it upward.

@taschaap
Copy link

taschaap commented Jun 7, 2021

These are great! Might I suggest adding an analytic signal function to the bunch?

@santisoler
Copy link
Member Author

I would just put in the comments that upward is also called vertical, but I do like your reasoning of calling it upward.

@ThomasMGeo do you mean in the comments or in the docstrings?

@ThomasMGeo
Copy link

I think either would work. Maybe just the comments to not clutter the doc strings?

@santisoler
Copy link
Member Author

I think either would work. Maybe just the comments to not clutter the doc strings?

I agree. I rather avoid using vertical in the docstring in order to avoid multiple ways of referring the same thing. So no problem of adding it to the comments.

Move the derivatives.py to the root of the package.
Ditch the transform module and create a new filters module.
Add the functions that apply the directional derivatives in the
frequency domain to a new filters/derivatives.py file.
Create small wrappers for the xrft.fft and xrft.ifft functions inside
the filters module, although I'm planning to keep them private for now.
Move the coordinates shifting to the ifft wrapper function.
Generalize the application of frequency domain filters into a single
private function harmonica.filters.utils.apply_filter
It's better to use xarray.differentiate for computing horizontal
derivatives of a regular grid than using FFT-based derivative.
We should add a tutorial explaining how to compute horizontal
derivatives.
@santisoler santisoler changed the title WIP Compute grid derivatives in the frequency domain WIP Compute upward derivative of a grid in the frequency domain Nov 10, 2021
@santisoler santisoler requested a review from leouieda May 20, 2022 18:49
@santisoler
Copy link
Member Author

I think this is ready as a first design for FFT transformations and functions. If we would like to change anything we can do it in the future, no big deal. After merging, next step would be to incorporate the FFT filters that @LL-Geo defined on #299 into this framework of filters + transformation functions.

I'm ignoring a minor CI error due to a small difference in tesseroids doctest. That issue is completely unrelated to this PR.

@santisoler santisoler merged commit cf4080c into main May 31, 2022
@santisoler santisoler deleted the fft-derivatives branch May 31, 2022 18:58
LL-Geo added a commit to LL-Geo/harmonica that referenced this pull request Jun 23, 2022
commit 54c3cbe52bd33a94ccbb5bb44f2958bb3afc9330
Author: LL-Geo <[email protected]>
Date:   Thu Jun 23 21:36:23 2022 +0800

    Update filter

    Add more filter

commit 4a5d6f1
Author: Santiago Soler <[email protected]>
Date:   Thu Jun 16 18:00:32 2022 -0300

    Avoid checking floats in tesseroid doctests (fatiando#326)

    Remove expected results for tesseroid calculations in docstring examples.
    Printing floats in forward modelling examples isn't that meaningful and often
    creates failures when running doctests: small differences between the expected
    and the got value could occur under some dependency and OS combinations.

commit cc697af
Author: Matt Tankersley <[email protected]>
Date:   Fri Jun 17 08:24:32 2022 +1200

    Add progressbar to prism forward gravity calculations (fatiando#315)

    Add optional `progressbar` flag to `prism_gravity` function and to the
    `gravity` method of the prism layer accesor to print a progress bar using
    `numba_progress`. Add `numba_progress` as optional dependency. Add test
    functions for the new feature.

commit 5a1c895
Author: Santiago Soler <[email protected]>
Date:   Tue Jun 14 13:20:39 2022 -0300

    Specify spherical latitude in point sources guide (fatiando#325)

    Replaces latitude for spherical latitude in another place of the
    `point.rst`. Fix typo on "Alternatively".

commit cb476b2
Author: Federico Esteban <[email protected]>
Date:   Tue Jun 14 11:33:22 2022 -0300

    Note that spherical and geodetic latitudes are equal in spherical ellipsoids (fatiando#324)

    Add sentence in the Coordinate Systems section of the User Guide noting that
    if the reference ellipsoid were a sphere both the spherical latitude and the
    geodetic latitude are equivalent.

commit 1256ff6
Author: Federico Esteban <[email protected]>
Date:   Mon Jun 13 11:35:02 2022 -0300

    Add Federico Esteban to AUTHORS.md (fatiando#323)

    Add his name, link to his GitHub account, affiliation and ORCID number.

commit 32de6e0
Author: Federico Esteban <[email protected]>
Date:   Thu Jun 9 15:44:48 2022 -0300

    Specify "spherical latitude" when describing coordinates of point masses (fatiando#321)

    Add "spherical" when describing the spherical latitude coordinate of point
    masses in the user guide. This way we differentiate it from the "latitude"
    geodetic coordinate.

commit 9667fab
Author: Santiago Soler <[email protected]>
Date:   Mon Jun 6 11:05:17 2022 -0300

    Fix small format errors in the user guide (fatiando#319)

    Fix link to EquivalentSources.predict method and fix superscripts in the docs.

commit 2f7fcb6
Author: Santiago Soler <[email protected]>
Date:   Fri Jun 3 11:17:51 2022 -0300

    Update docs and create a proper user guide (fatiando#305)

    Update Sphinx docs using sphinx-panels.
    Add a proper User Guide that will ultimately replace the gallery examples.
    Each page of the new User Guide is a .rst file that uses jupyter-sphinx to run example code blocks.
    Added pages for: Coordinate systems, Forward Modelling, Gravity corrections and Equivalent Sources.
    Added a new doc/versions.rst file with links to previous documentations.

commit cf4080c
Author: Santiago Soler <[email protected]>
Date:   Tue May 31 15:58:25 2022 -0300

    Compute upward derivative of a grid in the frequency domain (fatiando#238)

    Define a new derivative_upward function for computing the spatial upward
    derivative of a 2D grid in the frequency domain. The function makes use of
    xrft for handling Fourier transformations of xarray objects. Add a new
    filters subpackage that includes FFT filters: functions that take grids in
    frequency domain and return the desired filter also in frequency domain. Add
    fft and ifft wrapper functions of the xrft.fft and xrft.ifft ones. Add
    a new apply_filter function that takes a grid in the spatial domain, applies
    fft, the filter and ifft and returns the filtered grid also in spatial domain.
    Add tests for the new features and a gallery example for the upward derivative.
    Add netcdf4 as requirement for testing.

commit 6a30797
Author: Santiago Soler <[email protected]>
Date:   Fri May 27 16:22:18 2022 -0300

    Ditch soon-to-be deprecated args of equivalent sources grid method (fatiando#311)

    The grid() method of Verde gridders now take a coordinates argument with
    the coordinates of the target grid. The previous region, shape and
    spacing arguments will be deprecated in Verde v2.0.0. This change makes it
    easier for our equivalent sources classes: we don't need the extra upward
    argument, users can create the coordinates of the target grid using
    verde.grid_coordinates and pass them via coordinates argument. Ditch the
    upward, shape, spacing and region arguments from the equivalent sources
    gridders. Replace them for the new coordinates argument: users need to
    provide the coordinates of the target grid instead of building it through the
    grid method. Raise errors if any of those old arguments are being passed. Raise
    warnings if any kwargs are passed: they are being ignored and not passed to the
    BaseGridder.grid() method.

commit 51ceb7e
Author: Agustina <[email protected]>
Date:   Mon May 23 11:03:54 2022 -0300

    Remove deprecated point_mass_gravity function (fatiando#310)

    Remove point_mass_gravity function from harmonica because it was deprecated on
    PR fatiando#280. Remove related test functions.

commit f336aa8
Author: Santiago Soler <[email protected]>
Date:   Thu May 5 14:52:08 2022 -0300

    Drop support for Python 3.6 (fatiando#309)

    Remove the compatibility metadata, remove from the CI matrix, bump the
    python_requires to 3.7+.

commit d132abb
Author: Santiago Soler <[email protected]>
Date:   Tue May 3 12:47:22 2022 -0300

    Add computation of gravitational tensor components for point sources (fatiando#288)

    Add new kernel functions to compute gravity tensor components generated by
    point sources. Add test functions for the new feature: check that the diagonal
    elements satisfy the Laplace equation, compare all components against finite
    difference computations from the gravity acceleration. Add test class for
    checking the symmetry of tensor components. Refactor old test functions for
    point gravity: merge some functions into single ones through pytest
    parametrizations. Avoid using "gradient" for specifying the gravity
    acceleration vector: the "gravity gradient" is usually used to refer to the
    tensor.

commit eb71d54
Author: Santiago Soler <[email protected]>
Date:   Fri Apr 22 17:23:00 2022 -0300

    Add deprecations to datasets and synthetic modules (fatiando#304)

    Add FutureWarnings to public functions of the synthetic and dataset
    modules. Add tests for the new warnings. Both modules will be deprecated in
    Harmonica v0.6.0. Instead of providing sample datasets, Harmonica will depend
    on Ensaio for that. The synthetic surveys depend on some of the sample
    datasets, but those functions are intended to be used in methodology articles,
    so they should live somewhere else.

commit a4598ef
Author: Santiago Soler <[email protected]>
Date:   Fri Apr 22 17:06:43 2022 -0300

    Add conversion of prisms or a prism layer to PyVista objects (fatiando#291)

    Add a new visualization module that hosts prism_to_pyvista: a function to
    convert a set of prisms into a pyvista.UnstructuredGrid. Include the new
    module and this function in the API Reference. Add a new to_pyvista() method
    to the PrismLayer accessor that converts a prism layer into a pyvista grid,
    making it easier to plot it in 3D. The UnstructuredGrid has the information
    about each prism as hexahedrons, along with their physical properties as cell
    data. Add tests for the new features. Add pyvista and vtk as optional
    dependencies to environment.yml and setup.cfg. Add a new example for
    plotting a PrismLayer. Configure Sphinx to show pyvista plots in the gallery
    and to use the pyvista-plot directive in docstrings.

commit 762d210
Author: Santiago Soler <[email protected]>
Date:   Mon Apr 4 14:09:45 2022 -0300

    Update Black to its stable version (fatiando#301)

    Black has released a stable version: 22.3.0. Now the style check tests use this
    version. Fixes a bug on CI in which Black was trying to import a private module
    of click that doesn't exist anymore. Rerun black: now Black hugs simple
    power operators.

commit 10577fa
Author: Santiago Soler <[email protected]>
Date:   Mon Apr 4 14:00:42 2022 -0300

    Update Sphinx version to 4.5.0 (fatiando#302)

    Updates also sphinx gallery and sphinx book theme.
    This fixes a issue between latest jinja2 and Sphinx 3.5.*.

commit f880065
Author: Leonardo Uieda <[email protected]>
Date:   Fri Mar 18 13:34:51 2022 +0000

    Move configuration from setup.py to setup.cfg (fatiando#296)

    Make the move away from setup.py following the recommendations from the
    Python packaging guides. Moves the requirement listing to setup.cfg as
    well and will use a script to extract this for conda installing on CI.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants