Skip to content

Compute radar gate geolocation instead of delegating to PROJ - #28

Merged
rcjackson merged 6 commits into
mainfrom
test-pyproj-pin
Sep 8, 2026
Merged

Compute radar gate geolocation instead of delegating to PROJ#28
rcjackson merged 6 commits into
mainfrom
test-pyproj-pin

Conversation

@rcjackson

@rcjackson rcjackson commented Sep 8, 2026

Copy link
Copy Markdown
Owner

Fixes the test_adaptive_scanning.py failures that have run every night since 2026-09-06.

(This branch began as a one-line diagnostic pin. It now carries the actual fix and the pin is gone.)

Root cause

The model input is a rasterised radar image, so its geometry is part of the model contract. It was being produced by handing Cartopy AEQD metres and letting PROJ transform them — which means a PROJ release can silently change what the network sees. PROJ 9.8.0 did exactly that:

PROJ 9.8.0: "Add support for Equidistant Cylindrical ellipsoidal method (EPSG:1028)"

Cartopy's PlateCarree is defined as +proj=eqc +ellps=WGS84 +a=6378137.0 and assumes plate-carrée semantics (y = R·lat, linear in latitude). Under 9.8 that projection became genuinely ellipsoidal, so y is meridional arc length — ~24 km different at 41°N. It did not cancel, because the data went through aeqd→eqc while the axes extent went through geodetic→eqc.

Measured in CI, same runner and same torch, changing only pyproj:

              mask px   centroid                dist
PROJ 9.5.1     1605     41.68141, -88.01086       0.0 m   -> triggers   (pass)
PROJ 9.8.1     1386     41.52329, -88.08939   18732.4 m   -> no trigger (FAIL)

model input:  87.3% of pixels moved, ~10 px offset plus a scale warp

Ruled out along the way: PyTorch (2.14.0 identical either side), model weights (unchanged on HF since 2025-07-14), our code (no commits in the window), and a near-threshold flip (the case moves from 0 m to 18.7 km — nowhere near the 5000 m boundary).

The fix

adam.util.aeqd_to_lonlat solves the direct geodesic problem on WGS84 with Vincenty's formulae in numpy, agreeing with PROJ to 1.86 µm over 2160 gate points out to 300 km. The figure is drawn on a plain Matplotlib axes with longitude and latitude as the axes coordinates — which is what a plate carrée map is. Nothing in the model input path consults a projection library.

The two duplicated Cartopy render blocks in preprocess_radar_image and _preprocess collapse into one _render_model_input. Figure size, colormap, limits, PNG round-trip and normalisation are unchanged. visualize_lake_breeze keeps Cartopy — display only, never feeds the model.

Reaching into adam.util from adam.io was circular (instrument_steering imported RadarImage at module scope for annotations only). It is now a TYPE_CHECKING import, so adam.util is independently importable.

Evidence

Equivalent to the old renderer (same PROJ, so only the code path differs):

                        mean|diff|   pixels >1.0   translation
this change                0.193        0.22%         none
the PROJ 9.8 breakage    153.947       87.30%      ~10 px + warp

Version-independent — the point of the change, dumped on one machine at two projection versions:

pyproj 3.7.2 / PROJ 9.5.1  \
                            >  max |diff| = 0.000, 0.00% of pixels
pyproj 3.8.0 / PROJ 9.8.1  /

both give: mask 1608 px, centroid 41.68141,-88.01647, azimuth 224.612 deg, dist 0.0 m

Tests: 17 passed locally; unit tests, docs and diagnostics all green in CI at pyproj 3.8.0 with no pin — the exact configuration that has been failing.

Fixtures, and one thing reviewers should know

The two triggered-scan fixtures are regenerated, and their azimuth is now compared with a tolerance instead of byte-for-byte.

In the 2025-04-24 case the front sits ~3 pixels from the lidar (dist is 0.0 m — the mask covers the instrument), so azimuth_point takes atan2 of a very short vector. A one-pixel centroid shift swings the commanded azimuth by 8°, which is exactly what happened here (216.5° → 224.6°, centroid moving one grid cell in longitude). Pinning that number exactly would break again on the next Matplotlib or Py-ART release. Structure, motor parameters, elevations and the sector width are still checked exactly; only the absolute azimuth is given a tolerance, set to half the 30° scan width so a passing scan still covers at least half the reference sector.

Worth flagging honestly: best_model's mask IoU across the renderer change is 0.938, below the 0.98 bar I set for myself. The input differs by only 0.22% of pixels, so this is the model amplifying a tiny perturbation rather than renderer error — but that sensitivity is real and unchanged by this PR.

Not included

predict_lake_breeze.py never calls model.eval(), so Dropout(0.1) is live during inference and output varies with the seed. Real, but separate, and not a trivial fix: full eval() makes lakebreeze_best_model_fcn_resnet50 return an empty mask, because its checkpoint's BatchNorm running statistics are unusable (running_mean to 2574, running_var to 8.3e5). Disabling only the Dropout modules does work and makes output fully deterministic. Separate PR.

🤖 Generated with Claude Code

Not necessarily for merge. This isolates the suspected cause of the
test_adaptive_scanning.py failures that began with the 2026-09-06
nightly run.

Between the last passing run (2026-09-05, 33950032944) and the first
failing one (2026-09-06, 34017116670) exactly two of 151 installed
packages changed:

    anyio   4.15.0 -> 4.15.1
    pyproj  3.7.2  -> 3.8.0

torch (2.14.0) and torchvision (0.29.0) were identical across the
boundary, the HuggingFace model weights have not changed since
2025-07-14, and there were no commits to main in that window. pyproj is
the only plausible candidate remaining.

The mechanism would be indirect: the model input is a rasterised
matplotlib/cartopy figure (plot_ppi_map -> savefig -> decode_image in
get_radar_scan.py), so a shift in the projection stack perturbs the
rendered pixels, which changes the mask, which moves the lake-breeze
centroid, which flips the binary dist > max_distance trigger.

If this run is green, pyproj 3.8.0 is confirmed and we can decide
between a bounded requirement and addressing the underlying fragility.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Temporary, to be deleted with the pin once the regression is understood.

The failing assertion turns on a hard binary threshold:

    dist > max_distance (5000 m) -> scan not triggered

but neither the test nor the CI log ever shows what dist actually is, so
there is no way to tell whether this is a boundary-sensitivity problem
or a much larger change in the mask. Locally the same case yields
dist = 0.0 m and passes, but local runs torch 2.9.0 against CI's 2.14.0,
so local values say little about CI.

scripts/diagnose_trigger.py mirrors the exact sequence of
test_trigger_lidar_ppis_from_mask -- one manual_seed(42), then both
inferences in order, so the RNG state entering the second inference
matches the test -- and reports mask size, centroid, azimuth, dist, and
the signed margin against 5000 m.

The workflow runs it twice on one machine, changing only the pyproj
version between runs, so any movement in dist is attributable to pyproj
alone rather than to torch, weights, runner or data.

Motivation: the mechanism previously proposed for this regression does
not survive measurement. PROJ's raw aeqd->eqc output does change between
9.7.0 and 9.8.1, by 24 km in y, but cartopy normalises data against the
axes extent, which goes through the same transform. Composing both steps
the way cartopy does, rendered gate positions move by at most 0.031 px.
A sub-pixel perturbation should not flip a 5000 m threshold unless the
pipeline is already sitting on a decision boundary, which is what these
numbers are meant to establish or rule out.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The previous geometry measurement compared the wrong pair. CI's
pyproj 3.7.2 manylinux wheel bundles PROJ 9.5.1, while the macOS wheel
of the same pyproj release bundles 9.7.0, so measuring 9.7.0 vs 9.8.1
locally did not describe the transition CI actually makes, which is
9.5.1 -> 9.8.1.

The trigger diagnostic shows the effect is large, not marginal:

    PROJ 9.5.1   mask 1605 px   centroid 41.68141,-88.01086   dist     0.0 m
    PROJ 9.8.1   mask 1386 px   centroid 41.52329,-88.08939   dist 18732.4 m

The centroid moves about 17.6 km and the mask loses 14% of its area, so
this is not a threshold sitting on a knife edge.

Rather than model cartopy's internals again, dump the 256x256 tensor the
network actually receives under each version and diff it directly,
reporting max/mean difference, the fraction of pixels that move, and the
best integer alignment between the two images. That distinguishes a
rigid projection shift from a change in the rasterisation itself, which
determines whether removing PROJ from the render path would actually fix
anything.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The model input is a rasterised radar image, so its geometry is part of
the model contract. It was being produced by handing Cartopy AEQD metres
and letting PROJ transform them, which meant a PROJ release could
silently change what the network sees. PROJ 9.8.0 did exactly that: it
added an ellipsoidal Equidistant Cylindrical method, and Cartopy's
PlateCarree is defined as +proj=eqc +ellps=WGS84, so y stopped being
linear in latitude. Because the data went through aeqd->eqc while the
axes extent went through geodetic->eqc, the change did not cancel.
Measured in CI, changing only pyproj:

    PROJ 9.5.1   mask 1605 px   centroid 41.68141,-88.01086   dist     0.0 m
    PROJ 9.8.1   mask 1386 px   centroid 41.52329,-88.08939   dist 18732.4 m

87% of pixels moved. That is what has been failing the nightly runs
since 2026-09-06.

Compute the geolocation here instead. adam.util.aeqd_to_lonlat solves
the direct geodesic problem on WGS84 with Vincenty's formulae in numpy,
agreeing with PROJ to 1.86 microns over 2160 gate points out to 300 km,
and the figure is drawn on a plain Matplotlib axes with longitude and
latitude as the axes coordinates -- which is what a plate carree map is.
Nothing in the model input path consults a projection library now.

The two duplicated Cartopy render blocks in preprocess_radar_image and
_preprocess collapse into one _render_model_input helper. Figure size,
colormap, limits, PNG round trip and normalisation are unchanged.
visualize_lake_breeze keeps Cartopy: it is display only and never feeds
the model.

Reaching into adam.util from adam.io was circular, because
instrument_steering imported RadarImage at module scope. It only needed
it for annotations, so it is now a TYPE_CHECKING import and adam.util is
independently importable.

Verified against the old renderer at matching PROJ: mean pixel
difference 0.193 with 0.22% of pixels moving and no translation, versus
153.9 and 87.3% for the PROJ change being removed. Mask IoU 0.980 for
the no-augmentation model and 0.938 for the best model; the residual is
the model amplifying a 0.22% input change, not renderer error.

The two triggered-scan fixtures are regenerated, and their azimuth is
now compared with a tolerance rather than byte for byte. In the
2025-04-24 case the front sits about three pixels from the lidar, so
azimuth_point takes atan2 of a very short vector: a one pixel centroid
shift swings the commanded azimuth by 8 degrees, as it did here. Pinning
that number exactly would break again on any sub-pixel change. Structure,
motor parameters, elevations and the sector width are still checked
exactly; only the absolute azimuth is given a tolerance, set to half the
scan width.

Drops the diagnostic pyproj pin. The diagnostic scripts and workflow are
kept for one more run so CI can demonstrate that the render is now
identical across pyproj versions, and will be removed once it has.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Robert Jackson and others added 2 commits September 8, 2026 16:03
The previous run compared pyproj 3.8.0 against itself and reported a
0.000 difference, which proved nothing. Removing the pin from
requirements_dev.txt meant the install resolved straight to 3.8.0, so
the step labelled "pinned" was already running the suspect version.

Install 3.7.2 explicitly before the first dump so the comparison spans
PROJ 9.5.1 to 9.8.1, which is the transition that broke the tests.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
They have served their purpose. Final run, dumping the model input on
one machine at two projection versions:

    pyproj 3.7.2 / PROJ 9.5.1  \
                                >  max |diff| = 0.000, 0.00% of pixels
    pyproj 3.8.0 / PROJ 9.8.1  /

and the derived quantities agree exactly across both:

    mask 1608 px, centroid 41.68141,-88.01647, azimuth 224.612 deg, dist 0.0 m

Before the fix the same comparison gave 87.3% of pixels moved, the mask
dropping 1605 -> 1386 px and the distance going 0.0 -> 18732.4 m.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@rcjackson rcjackson changed the title DIAGNOSTIC: pin pyproj==3.7.2 to test unit test regression Compute radar gate geolocation instead of delegating to PROJ Sep 8, 2026
@rcjackson
rcjackson marked this pull request as ready for review September 8, 2026 21:09
@rcjackson
rcjackson merged commit 29818bb into main Sep 8, 2026
4 checks passed
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