Repository navigation
Conversation
The law of cosines on the two ranges returned NaN where the baseline is nearly parallel to the look (|cos| > 1), and was biased by the range of the secondary taken before its along-track shift was removed. The ranges are kept in the signature for callers such as disp-s1. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Change Summary
baseline.computegot the perpendicular baseline from the two ranges by the law of cosines, thensin_theta = np.sqrt(1 - cos_theta**2). That fails in two ways:|cos_theta|just above 1 and the square root returns NaN. In DISP-S1 products this shows up as a NaN band incorrections/perpendicular_baselinealong the B⊥ = 0 line. In one set of 376 archived products, 15 have it, 20k–2.7M pixels each.compute_baselines(and disp-s1's_baselines.compute_baselines) removes the along-track offset from the secondary position, but passes the secondary range measured before that shift. On S1A–S1B pairs this is a 0.1–0.5 mm range mismatch, which the law of cosines turns into centimetres of B⊥ error near zero: up to 19 cm at a 32 m along-track shift.The fix computes B⊥ directly as the baseline component perpendicular to the unit look vector,
|B × l̂|. It doesn't use the ranges, so neither problem can occur. The sign rule is unchanged.ref_range/sec_rangestay in the signature, now unused, because disp-s1 callsdolphin.baseline.computewith them positionally.Away from zero nothing changes: on
tests/data/baseline_cslcs(B⊥ ≈ 135–142 m) the new values are within 0.04 mm of the old ones.Verification on archived DISP-S1 products
For each of the 15 affected products, B⊥ was recomputed through disp-s1's own code (
disp_s1._baselines.compute_baselineson the product grid every 50th pixel, then_interpolate_data, as inproduct.py). The inputs were the first reference and secondary CSLC by name from ASF, run once with this branch and once withmain:mainthe recomputation reproduces every archived layer: the same NaN pixels and values within 0.03 mm. So the NaNs come from this function and not from anything downstream.Top: an S1A–S1A pair, where the NaN band disappears. Bottom: an S1A–S1B pair with a 21–32 m along-track shift, where the band disappears and the bias away from the zero line is removed too. In the figure titles, "where |B⊥| > 0.5 m" is measured on the fixed layer; in the table below it's measured on the archived layer.
Same comparison on one burst (
t035_073233_iw3, 20160705 → 20160729), on the CSLC lon/lat grid:How to verify
The new tests put the secondary 100 m or 1000 m along the look and 10 m to 0 m across it, and check that
|B⊥|is recovered to 1e-6 m and is finite. Onmainthey fail at 0.01 m and 0 m. A second test checks that the sign flips with the side.Pre-flight:
pre-commit run --all-filesclean.pytest: 781 tests run (tests/test_io_paths.pyskipped,motonot installed): 757 passed, 21 skipped, 24 failed. The same 24 also fail onmainin that environment (phassnot installed,gdal_merge.pynot on PATH), so none come from this change.Not in this PR
test_baseline_computehas two barepytest.approx(...)lines that never assert, and its expected minimum (134.98) doesn't match what the function returns onmain(134.51). That needs a separate fix.Related issue number
None.
Checklist
🤖 Generated with Claude Code