Repository navigation
FIX: IRIS beam-center azimuth for counter-clockwise rays - #530
Conversation
IrisRawFile.get_moment unwrapped a ray only when start - stop > 5, which handles a clockwise antenna only. A counter-clockwise ray across north (start 0.5, stop 359.6) was placed at about 180 degrees, and so was any counter-clockwise ray wider than 5 degrees. IRIS sector sweeps alternate direction (Programming Guide, task_ppi_scan_info), so such rays occur. _azimuth_centers unwraps the shorter way round when start and stop are more than half a circle apart. For clockwise data the arithmetic is the same as before, so existing azimuths are bit-identical. Closes openradar#526
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #530 +/- ##
=======================================
Coverage 96.03% 96.04%
=======================================
Files 30 30
Lines 7494 7503 +9
=======================================
+ Hits 7197 7206 +9
Misses 297 297
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
With rawdata=True the ray-header angles are raw BIN2 counts, not degrees, so the degree thresholds of _azimuth_centers do not apply; keep the previous arithmetic there (azimuths byte-identical to main). Fold negative centers before the >= 360 check: a center closer to 0 than half an ulp otherwise rounded up to exactly 360.0 (e.g. stop = nextafter(360, 0)). Use "center" throughout the history entry.
syedhamidali
left a comment
There was a problem hiding this comment.
Thanks @aladinor, nice fix. I checked it locally: the new tests fail on main and pass here, and azimuth and elevation are identical to main on the IRIS sample files I have (cor-main, SUR and RAW_NA, 23 sweeps). I also ran _azimuth_centers on 200000 random rays in both directions, including across north: it matches the true midpoint, stays in [0, 360), and gives the same center when start and stop are swapped. One thing to change (the changelog), two small notes below.
|
@aladinor Thanks for getting this fixed for CCW here. I'd really like to see this generalized for all backends which have similar approaches. So +1 for opening issues. |
Move the history entry to the unreleased section and note the raw-mode and 180 degree limits of the beam center calculation.
|
Thanks for the review @syedhamidali. All three comments are addressed in 93e77f2, which also merges current main:
#459 needs the same fix when it syncs with main. Its virtual reader has its own midpoint helper, The same problem in the other backends is tracked in #569 (ODIM), #570 (GAMIC) and #571 (Rainbow). |
history.mdWhat this does
IrisRawFile.get_momentcomputes each ray's beam-center azimuth as the start/stop midpoint, and unwrapped only whenstart - stop > 5:That covers a clockwise antenna only. For a counter-clockwise one, two cases came out 180° off:
Counter-clockwise rays do occur: the IRIS Programming Guide (M212927EN-B,
task_ppi_scan_info) notes that sector sweeps alternate in direction.The new
_azimuth_centers(start, stop)unwraps the shorter way round when start and stop are more than half a circle apart, in either direction. For clockwise data the arithmetic is the same as before, so existing azimuths don't change: azimuth and elevation are byte-identical tomainon 27 sweeps from 9 files (both fixtures, 6 IDEAM volumes from 3 sites, and the Arouca file).Tests
In
tests/io/test_iris.py, all failing onmain:test_azimuth_centers: clockwise and counter-clockwise, each normal and across north (east and west of north), a wide counter-clockwise ray, and exactly north.test_azimuth_centers_stay_below_360: the result stays in [0, 360).test_counter_clockwise_sweep_keeps_beam_centers: no counter-clockwise file is available (none in the fixtures or in IDEAM samples), so this simulates one on SUR by swapping each ray's start and stop azimuth. Every beam center must stay put. The test asserts that the swap really happened and that the fixture has a ray across north.The smoke test from #526 now exits 0. The full suite passes locally (1380 passed, 19 skipped).
Related, not changed here
Running the same check on the other backends' midpoint formulas (simulated sweeps):
rainbow.py) uses the samestart - stop > 5rule: 1 counter-clockwise ray off by 180°._get_azimuth_how) unwraps onstop < start, so a counter-clockwise sweep has every ray off by 180°._get_azimuth) gives every counter-clockwise ray an 180° offset, and also returns 360.0 for the clockwise ray across north (it never folds).I can open issues for these. The helper could move to
common.pyand be shared.#459 (virtual IRIS) already uses a wrap-aware rule. Once this is in, it can call
_azimuth_centerstoo.AI usage
This PR was developed with Claude Code, using Anthropic's Claude Opus 5.5 model (
claude-opus-5-5). I iterated with the agent: it wrote the code and tests at my direction, and also ran the smoke tests, an adversarial review and code audits. I have read and validated the changes and can explain any line of them.