Fix north-south flip in FFT filters for descending-coordinate grids - #683
Fix north-south flip in FFT filters for descending-coordinate grids#683gaoflow wants to merge 1 commit into
Conversation
The FFT-based filters (derivatives, upward continuation, reduction to pole, etc.) silently produced an output flipped along a dimension when the input grid stored that coordinate in descending order, as happens for grids read from some file formats (e.g. ERMapper/ERS) where northing decreases with row index. The inverse FFT always returns the grid in ascending coordinate order, but apply_filter then assigned the original (descending) coordinates onto that ascending result, mislabelling every row/column and flipping the data relative to its coordinates. The frequency coordinates built by xrft also assume positive sample spacing. Sort the grid to ascending order before transforming and restore the original coordinate order afterwards via reindex. Grids already in ascending order are unaffected. Adds a parametrised regression test (northing and easting) asserting a descending-coordinate grid is not flipped. Fixes fatiando#586
|
Hi @gaoflow. Thanks for opening the PR. I'm suspecting this contribution made significant use of generative AI software to produce the code and the PR description text. Could you confirm this is the case? Could you disclose how generative AI software was used in the process of opening this PR? |
|
Yes — you're right, and thanks for asking directly. I work with an AI coding agent (Anthropic's Claude). It helps me locate and reproduce the bug, drafts the patch and the tests, and drafted this PR description. It runs under my GitHub account and my direction: I review the diff before it goes out, the tests are run locally, and I'm accountable for what's submitted. The mismatch numbers in the table are from real runs, not written prose. The description is also too long, which is the other half of what you picked up on. That one's a fair hit — I'll keep them shorter. If Harmonica or Fatiando would prefer not to accept AI-assisted contributions, or wants the disclosure recorded in the commit message or changelog, say which and I'll comply — including closing this if that's the call. #586 is your issue and the fix is four lines, so nothing is lost if you'd rather write it yourself. |
Fixes #586
Problem
The FFT-based filters (
derivative_*,upward_continuation,reduction_to_pole, etc., all routed throughapply_filter) silently produced an output flipped along a dimension when the input grid stored that coordinate in descending order. This happens for grids read from some file formats (e.g. ERMapper/ERS) where the northing decreases with row index. @santisoler reproduced this in the issue.Root cause
In
harmonica/filters/_utils.py::apply_filter, the inverse FFT (xrft.ifft) always returns the grid in ascending coordinate order. The code then did:assigning the original (descending) coordinates onto the ascending result — mislabelling every row/column so the data ends up flipped relative to its coordinates. (The frequency coordinates built by
xrftalso assume positive sample spacing.)Fix
Sort the grid to ascending order before transforming, and restore the original coordinate order afterwards via
reindex. Grids already in ascending order take the same path as before (nosortby/reindex), so their results are unchanged.Reproduction / verification
A minimal synthetic check (no external data needed): apply a filter to a grid and to the same physical grid with one coordinate reversed; after aligning on that coordinate the two results must be identical.
derivative_upwardupward_continuationreduction_to_polenorthingandeasting) intest/test_transformations.pythat fails onmainand passes with the fix.test/test_transformations.py+test/test_filters.pysuite: 62 passed (was 60).ruff check/ruff format --checkclean.