Skip to content

Add points to geostationary bounding box when determining subset slices - #757

Open
djhoese wants to merge 2 commits into
pytroll:mainfrom
djhoese:feat-densify-geos-boundary
Open

djhoese wants to merge 2 commits into
pytroll:mainfrom
djhoese:feat-densify-geos-boundary

Conversation

@djhoese

@djhoese djhoese commented Oct 8, 2026

Copy link
Copy Markdown
Member

This is an extension of #729 that applies the same idea to general get_area_slices. I had been working with Claude on reviewing #729 and #700 as they deal with similar issues. Claude pointed out flaws in #700 and suggested that until spherical intersection calculations in pyresample are better, #729's solution would be a clean replacement for what #700 is doing.

Claude wrote this code. I asked it to make a better suite of tests that borrowed from the assertions in #700, but with more cases. One of the flaws of #700 is that it really only makes the calculations better where the target area is completely inside the source area. I wanted more cases.

I don't think #700 should be merged and instead #729 and this PR should be merged. I'll have Claude update this PR with a shared helper function once I merge #729.

  • Closes #xxxx
  • Tests added
  • Tests passed
  • Fully documented

@codecov

codecov Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.98%. Comparing base (4a46786) to head (4662c4b).
⚠️ Report is 15 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #757      +/-   ##
==========================================
+ Coverage   93.90%   93.98%   +0.08%     
==========================================
  Files          89       89              
  Lines       13924    13989      +65     
==========================================
+ Hits        13075    13148      +73     
+ Misses        849      841       -8     
Flag Coverage Δ
unittests 93.98% <100.00%> (+0.08%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@djhoese

djhoese commented Oct 8, 2026

Copy link
Copy Markdown
Member Author

Performance numbers from Claude:

Overall the PR is about 1.5× slower than main across these cases, and full-disk sources are unchanged. Partial-disk sources are 1.1–3.3× slower, but in 7 of the 9 partial-disk cases main's faster answer was a slice that was too small (or an error).

The table compares main (94eabc2) with your PR commit (c4da9ce). Each time is the median of 7 runs, and the "missing pixels" column comes from those same two commits. It's in a code block so the raw markdown survives copying, and it's also saved as pr_perf_table.md in my scratchpad (/tmp/claude-53807/-home-davidh-repos-git-pyresample/2c77a686-8f1c-4596-acbb-2a73545294c7/scratchpad/pr_perf_table.md).

Case Source outline vertices main This PR Change Missing pixels (main → PR)
target_inside_full_disk 50 → 50 229 ms 239 ms 1.04× 1 → 1
target_inside_partial_disk 22 → 37 153 ms 178 ms 1.16× 272 → 1
target_crosses_clipped_edge 22 → 37 99 ms ¹ 191 ms 1.92× NotImplementedError → 0
target_crosses_clipped_edge_and_limb 22 → 37 42 ms 75 ms 1.79× 17 → 2
target_crosses_two_clipped_edges 11 → 22 54 ms 101 ms 1.85× 16 → 0
target_crosses_clipped_edge_and_opposite_limb 11 → 22 48 ms 77 ms 1.61× 85 → 0
target_contains_source 11 → 22 43 ms 78 ms 1.82× 0 → 0
target_crosses_limb 50 → 50 196 ms 195 ms 1.00× 2 → 2
target_contains_full_disk 50 → 50 343 ms 343 ms 1.00× 5 → 5
target_is_other_partial_disk ² 22 → 37 114 ms 370 ms 3.26× 319 → 16
RSS → euro4, 1024×1024 (#728) 22 → 37 305 ms 337 ms 1.10× 273 → 1
RSS → eurol, 2560×2048 22 → 37 386 ms 863 ms 2.24× 549 → 0
Total 2012 ms 3048 ms 1.51×

Times are the median of 7 calls to AreaDefinition.get_area_slices with slice caching off, comparing main (94eabc2) and this PR (c4da9ce) on the same machine. The first ten rows are the cases in the new test_get_area_slices_geos_cross_projection_coverage test. "Missing pixels" is the largest number of needed source rows or columns cut off on any side, where "needed" means hit by at least one target pixel center.

¹ Time until main raises NotImplementedError.
² The target is also a partial geostationary disk, so both outlines gain vertices.

Full-disk sources are unchanged because their outline is identical. Partial-disk sources are 1.1–3.3× slower. Nearly all of the time is in the pure-Python spherical polygon intersection, which slows down as vertices are added.

@djhoese
djhoese force-pushed the feat-densify-geos-boundary branch from c4da9ce to 4662c4b Compare October 8, 2026 20:44
@djhoese

djhoese commented Oct 8, 2026

Copy link
Copy Markdown
Member Author

I've now rebased this branch and moved the work in #729 to use the same functionality that was used in this PR. Pretty much the same logic just a little more flexible. Now the two slicing code branches use the same code for geostationary segmenting.

@djhoese

djhoese commented Oct 9, 2026

Copy link
Copy Markdown
Member Author

Here's an ABI CONUS case that Claude remapped using nearest neighbor to a lon/lat grid.

goes16_conus_get_area_slices_before_after

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant