Skip to content

Mrb 807 avoid regridding forecasts#130

Open
Louis-Frey wants to merge 4 commits into
mainfrom
MRB-807-avoid-regridding-forecasts
Open

Mrb 807 avoid regridding forecasts#130
Louis-Frey wants to merge 4 commits into
mainfrom
MRB-807-avoid-regridding-forecasts

Conversation

@Louis-Frey
Copy link
Copy Markdown
Contributor

Performance bottleneck in map_forecast_to_truth

Date: 2026-04-02

Context

During testing of the spatial verification pipeline (evalml experiment --spatial)
with a single-init-time config (forecasters-ich1-oper-fixed.yaml, 2025-03-01),
the verif_metrics_spatial_baseline jobs for ICON-CH1-EPS took ~20 minutes despite
having only one init time to process — i.e., nothing to aggregate.

Root cause

The bottleneck is in src/verification/spatial.py: map_forecast_to_truth().

Even when the forecast (ICON-CH1-EPS) and truth (KENDA-CH1) are on the same
1km grid and no actual remapping is needed, the function always:

  1. Stacks (y, x) into a flat 'values' dimension (~1M+ points)
  2. Builds a cKDTree over all source (forecast) lat/lon points -- O(N log N)
  3. Queries the tree for all target (truth) lat/lon points -- O(M log N)

At 1km resolution this involves ~1M grid points, making the kd-tree operations
the dominant cost regardless of how many init times are processed.

There is already a TODO comment in the code acknowledging this:

TODO: return fcst unchanged when forecast and truth are already aligned

(src/verification/spatial.py, line 124)

Recommended fix

Before building the kd-tree, check whether forecast and truth lat/lon
coordinates are already aligned (e.g. same shape and max abs difference
below a small tolerance). If so, return fcst unchanged immediately.

This would make the baseline spatial verification near-instantaneous for
same-grid configurations (ICON-CH1-EPS vs KENDA-CH1), and would also
benefit any other same-grid run/truth combination.

…y aligned

Avoids O(N log N) kd-tree build and query (~1M points at 1km resolution)
when forecast and truth share the same grid, reducing baseline spatial
verification from ~20 minutes to near-instantaneous.
@Louis-Frey Louis-Frey requested review from dnerini and frazane April 7, 2026 09:34
Comment thread tests/unit/test_spatial_mapping.py
@jonasbhend
Copy link
Copy Markdown
Contributor

Ok, @frazane @Louis-Frey @dnerini I have expanded the tests slightly to make sure we can align forecasts even when skipping interpolation. Can someone of you have a close look at the tests to make sure we have caught all the edge cases.

@jonasbhend jonasbhend requested review from clairemerker, dnerini and frazane and removed request for dnerini and frazane June 1, 2026 07:25
@jonasbhend
Copy link
Copy Markdown
Contributor

jonasbhend commented Jun 2, 2026

@clairemerker if you have time to look into this, your feedback would be very much appreciated. For context, I worry about the edge cases that render alignment of forecast and ground truth with xarray impossible:

  1. ICON grid with rounding errors in lon/lat
  2. ICON grid with corresponding lon/lat coordinates but different 'values' dimension indices

I don't know if the latter could occur. Clearly if stored as anything other than int, the dimension values could also be affected by rounding errors, which I don't think is a thing, but anyway...

@dnerini
Copy link
Copy Markdown
Member

dnerini commented Jun 2, 2026

I worry about the edge cases

but we essentially work with always the same 2 or 3 models grids, can't we just test each of them to make sure it works?
and in the worst case, what will happen is that we'll be regridding some data needlessly, which is not such a big deal, no?

@jonasbhend
Copy link
Copy Markdown
Contributor

I worry about the edge cases

but we essentially work with always the same 2 or 3 models grids, can't we just test each of them to make sure it works? and in the worst case, what will happen is that we'll be regridding some data needlessly, which is not such a big deal, no?

Agreed, I think the key point is that skipping the interpolation when coordinates are the same withing rounding errors is unsafe, as this potentially may result in empty xarray joins after alignment. The current implementation tests for that so we should be good to go. I don't think we need to test the different grids explicitly, as this would complicate the unit tests or bloat the repos (we'd have to check in the grid specifications, right)?

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.

3 participants