Skip to content

Fix division-by-zero in GraphWeatherForecaster grid mapping for single-axis grids - #231

Open
munzzyy wants to merge 1 commit into
openclimatefix:mainfrom
munzzyy:fix/grid-mapping-single-axis
Open

munzzyy wants to merge 1 commit into
openclimatefix:mainfrom
munzzyy:fix/grid-mapping-single-axis

Conversation

@munzzyy

@munzzyy munzzyy commented Jul 5, 2026

Copy link
Copy Markdown

GraphWeatherForecaster.__init__ calls _create_grid_mapping unconditionally, which divides by max(unique_lats) - min(unique_lats) and max(unique_lons) - min(unique_lons) with no guard for the degenerate case where every node shares one latitude or one longitude. When that happens the range is zero and it raises ZeroDivisionError before the model even finishes constructing.

Repro:

from graph_weather.models.forecast import GraphWeatherForecaster

GraphWeatherForecaster(lat_lons=[(45.0, 0.0), (45.0, 10.0)])
# ZeroDivisionError: float division by zero

Same thing happens with a single unique longitude, or a single point (both axes degenerate at once).

Fix: when an axis has no spread to normalize against, every node on that axis maps to row/col 0 instead of dividing by zero. Normal 2D grids go through the same math as before, so nothing changes for the common case.

Added two tests, test_forecaster_single_lat and test_forecaster_single_lon, covering both degenerate axes and confirming the model still runs a forward pass without producing NaNs.

I don't have a GPU/full env set up locally to run the whole suite, so I verified the fixed arithmetic in isolation against a range of inputs (single-lat, single-lon, single-point, and a regular grid checked against the original formula for equivalence) and ran ruff/black against the touched files, both clean.

…rids

GraphWeatherForecaster._create_grid_mapping divided by
(max(unique_lats) - min(unique_lats)) and the same for longitudes, with
no guard for the degenerate case where all lat_lons share one latitude
or one longitude. That range hits zero and raises ZeroDivisionError,
so any 1D grid crashes right in __init__.

Fall back to row/col 0 when the axis has no spread to normalize
against. Normal 2D grids are unaffected.
@munzzyy

munzzyy commented Jul 5, 2026

Copy link
Copy Markdown
Author

Same failing checks as #230, so I compared notes.

pytest (macos + ubuntu): both crash during collection before any test body runs - torch_scatter's native extension aborts/segfaults while torch_geometric/typing.py is being imported (macOS: Fatal Python error: Aborted, std::length_error: vector; Ubuntu: Segmentation fault), triggered by loading tests/test_fgn.py, which this PR doesn't touch. Checked main's CI history for the "Python package" workflow and it's been red the same way since June 4, well before this branch existed - so this is a pre-existing pixi/torch-stack issue, not something introduced here.

pre-commit.ci: the ruff hook fails on files in graph_weather/models/aurora/, graph_weather/models/cafa/, and graph_weather/data/anemoi_dataloader.py. I ran ruff==0.15.15 (matches .pre-commit-config.yaml) against a clean checkout of main and got the identical 156 errors in the identical files. Ran it against this branch too - same 156, so this diff doesn't add any new violations (the two new test functions lack docstrings, same as every other test function already in tests/test_model.py - consistent with the existing file, not a new issue).

Verified the fix itself works: extracted _create_grid_mapping's logic and ran it standalone against a single-unique-latitude grid, a single-unique-longitude grid, and a normal 2D grid. Degenerate axes now map every node to row/col 0 instead of raising ZeroDivisionError, and the normal case is unchanged. Matches what test_forecaster_single_lat/test_forecaster_single_lon assert.

Neither failing check here is caused by this branch - it's the same upstream CI breakage as #230.

@munzzyy

munzzyy commented Jul 5, 2026

Copy link
Copy Markdown
Author

Filed #232 for the segfault so it's tracked separately from this PR.

@munzzyy

munzzyy commented Jul 6, 2026

Copy link
Copy Markdown
Author

Checked this before pushing further changes. Both pytest jobs die during collection, before any test in this diff runs:

Fatal Python error: Segmentation fault
  File ".../torch_scatter/__init__.py", line 16 in <module>
  File ".../torch/_ops.py", line 1442 in load_library

Same crash on macOS and Ubuntu, and it happens just importing graph_weather, nothing to do with graph_weather/models/forecast.py or tests/test_model.py.

Confirmed against main: identical segfault, same frame, on the latest push to main (run https://github.com/openclimatefix/graph_weather/actions/runs/28750434951). Looks like a torch_scatter/torch/pyg_lib ABI mismatch in the pixi-resolved CPU environment, not something introduced here.

pre-commit.ci is the same situation. I ran pre-commit run --all-files locally on main and got 156 pre-existing ruff errors in files unrelated to this PR (aurora/, cafa/, weathermesh/, fengwu_ghr/, train/, setup.py). The two files this PR actually changes come back clean.

Flagging so this isn't read as a problem with the fix itself — will rebase once the pytest/torch_scatter breakage is sorted upstream.

@munzzyy

munzzyy commented Jul 6, 2026

Copy link
Copy Markdown
Author

The failures here are inherited from main: pytest has segfaulted on every main run since early June (#232, same Fatal Python error: Segmentation fault loading torch), and the ruff errors in the pre-commit run are all in anemoi_dataloader.py and the new aurora/cafa modules, none of which this PR touches. I ran ruff 0.15.15 with the repo config over the files changed here and they come back clean. I'll rebase once main is green.

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.

1 participant