Compute horizontal derivatives of grids using finite differences #378
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Use a central finite-difference method by default to compute the horizontal derivatives of a regular grid. Uses the Xarray's DataArray.differentiate method.
Refactor private function to make it clearer how it works.
The second order upward derivative in FFT has a lot of error in comparison with the one obtained by summing the two horizontal derivatives done with finite differences.
…a into horizontal-finite-diff
|
@LL-Geo, I'm planning to merge this PR during this week. If you have the chance to take a look at it, that would be nice. Otherwise, don't worry, we can always change this code afterwards. Since horizontal derivatives using finite differences in the grid are more accurate than their FFT counterpart, I think it might be better to offer the finite differences option as the default one. This was also the default behavior in the old |
With if and elif statements, pytest wasn't able to determine if we were fully testing these functions, since invalid method were being caught by the private check function.
Remove the private functions that just checked a valid method and repeat the error raising inside each horizontal derivative function. This simplifies code to a simple "if, elif, else" block.
|
@LL-Geo, I'm merging this so I can start the release of Harmonica. Don't worry about the review. If you find something we should change, feel free to open an issue or a pr! Thanks again for all the work with the fft filters! |
|
Just had a quick look! That looks great! 👍 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Make functions
derivative_eastingandderivative_northingto compute the horizontal derivatives using central finite-differences through thexarray.DataArray.differentiatemethod by default. Add amethodargument to let users choose if they want to use the FFT implementation or the finite differences one. This implementation mimics the one we had in the oldfatiando.