graham/prop
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions
prop/bugs.md
Graham McIntire 6aa91e7656
fix: April 2026 codebase review — address 13 bugs across propagation chain 
Each fix is covered by a regression test that fails on `main` and
passes on this commit.

Round 1 (initial review):

* propagation: thread `latitude` into the conditions map so
  `score_season/4` actually picks up regional multipliers
* hrrr_client / fetcher.rs: `nearest_hrrr_hour` rounds DOWN, never at
  a future cycle that NOAA hasn't published yet
* radio: spherical-vector great-circle midpoint replaces the
  arithmetic mean — anti-meridian paths no longer fold to Greenwich
* weather: `reconcile_weather_statuses` scales the longitude band by
  `1 / cos(lat)` so the bbox stays ~150 km wide at every latitude
* radio/maidenhead: clamp 90°/180° below the field-bucket overflow so
  `from_latlon` never emits invalid characters like 'S'
* prop_grid_rs/pipeline: merge HRRR + NEXRAD-derived rain rates and
  read `best_duct_freq_ghz` into `best_duct_band_ghz` so the Native
  Duct Boost actually fires
* propagation/region (Elixir + Rust): inclusive upper bounds so points
  exactly at lat_max get the regional multiplier
* weather/sounding_params (Elixir + Rust): drop the 10 m gradient
  floor so HRRR's thin near-surface layers stop hiding sharp ducts
* weather/sounding_params: when the profile ends inside a duct,
  finalize it with the highest sample as the top instead of throwing
  it away (Rust port already correct)

Round 2 (post-fix sweep):

* radio + commercial: single canonical haversine in Radio (atan2
  form); Commercial delegates instead of carrying a second copy that
  could disagree at threshold distances
* prop_grid_rs/profiles_file: `snap_coords` matches Elixir's
  step-aware snap (`round(coord/0.125) * 0.125`, then 3-dp round) so
  Rust-keyed and Elixir-keyed profile maps land on the same cell
* weather/grib2/wgrib2: `parse_lon_val_segment` uses `Float.parse`
  uniformly — wgrib2 dropping the trailing `.0` from a longitude no
  longer crashes the whole chain step
2026-04-25 10:52:51 -05:00

3 KiB
Raw Blame History

Identified Bugs and Observations (Post-Fix Review)

This document reflects the state of the codebase after the fixes for the initial 15 bugs were applied.

All three items below were verified, covered by failing tests, and fixed on 2026-04-25.

1. Haversine Implementation Inconsistency — FIXED 2026-04-25

Files: lib/microwaveprop/radio.ex vs lib/microwaveprop/commercial.ex Description:

  • Radio.haversine_km/4 uses the asin form: 2 * r * :math.asin(:math.sqrt(a))
  • Commercial.haversine_km/4 uses the atan2 form: 2 * r * :math.atan2(:math.sqrt(a), :math.sqrt(1 - a)) Impact: While functionally similar, atan2 is the industry standard for floating-point stability at large distances. Having two different implementations for the same physical calculation can lead to tiny discrepancies (sub-millimeter) that might cause a contact or commercial link to intermittently cross a distance threshold (e.g., 75 km) depending on which module performs the check. Fix: Switched Radio.haversine_km/4 to the atan2 form and made Commercial's private wrapper delegate to it. One canonical implementation, no per-module drift. Numerical-stability regression test added in test/microwaveprop/radio_test.exs.

2. Grid Snapping Mathematical Drift — FIXED 2026-04-25

Files: lib/microwaveprop/propagation/profiles_file.ex vs rust/prop_grid_rs/src/profiles_file.rs Description:

  • Elixir snap/2: Float.round(Float.round(lat / step) * step, 3)
  • Rust snap_coords: (lat * 1000.0).round() / 1000.0 Impact: The Elixir version performs a division and multiplication by the grid step (0.125) before rounding. The Rust version rounds the raw coordinate to the third decimal place. For coordinates that land exactly on a 0.0625 boundary (the halfway point between cells), these two methods may snap to different cells, causing the UI to report "No Data" or show the wrong factor breakdown for a clicked point. Fix: Rust snap_coords now mirrors the Elixir step-aware snap (round(coord / 0.125) * 0.125, then 3-decimal round). Cross-stack parity test in rust/prop_grid_rs/src/profiles_file.rs enumerates known-disagreeing inputs.

3. Potential Exception in Wgrib2 Parsing (Theoretical) — FIXED 2026-04-25

File: lib/microwaveprop/weather/grib2/wgrib2.ex Description: parse_lon_val_segment/1 uses String.to_float(lon_str) for the longitude, but Float.parse for latitude and value. Impact: If wgrib2 were to ever return an integer string for longitude (e.g. "235") instead of a float ("235.0"), String.to_float would raise an ArgumentError. While wgrib2 typically outputs floats, using Float.parse would be more consistent with the other fields in the same line. Fix: All three numeric fields now go through Float.parse/1 (in a with chain so a malformed segment returns nil instead of bringing down the chain step). Function exposed as @doc false and covered by direct unit tests for both decimal-formatted and integer-formatted longitude strings.