🩹 SimpleSDMLayers handling of clip with coordinates on cell boundaries #153

tpoisot · 2023-02-18T01:45:42Z

Do not use stride when clipping
- Closes Avoid using stride when clipping #150
Add options to clip to decide how to expand
- Closes Add expand options to clip #151
Test that clipping works with coordinates that are at the border of two pixels
- Closes Add a test for clipping with exact coordinates #149
- Redirects Test for clipping with natural coordinates #152 to point to this branch
Closes 🐛 Update the stitching code to get the right bounding box #155
- Closes Stitch returns the wrong bounding box in the end #154

github-actions · 2023-02-18T01:45:56Z

Documentation for this PR at https://poisotlab.github.io/SpeciesDistributionToolkit.jl/previews/PR153/

tpoisot · 2023-02-18T02:41:14Z

OK, so this is a version that (i) does not use stride (ever) and (ii) uses expand to decide which side pixels to include.

@gabrieldansereau I think the next step is to write a bunch of tests to be sure this works, and then we can merge/tag. Specifically, the code in this PR uses the default of not including the cells around the limit, but this can be called by adding expand=[:left,:right,:bottom,:top] in the tests.

codecov-commenter · 2023-02-18T02:52:32Z

Codecov Report

Base: 47.49% // Head: 54.00% // Increases project coverage by +6.51% 🎉

Coverage data is based on head (bf98b78) compared to base (a0970e1).
Patch coverage: 81.39% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #153      +/-   ##
==========================================
+ Coverage   47.49%   54.00%   +6.51%     
==========================================
  Files          36       47      +11     
  Lines        1196     1511     +315     
==========================================
+ Hits          568      816     +248     
- Misses        628      695      +67

Flag	Coverage Δ
unittests	`54.00% <81.39%> (+6.51%)`	⬆️

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

Impacted Files	Coverage Δ
SimpleSDMLayers/src/operations/tiling.jl	`0.00% <0.00%> (ø)`
SimpleSDMLayers/src/lib/clip.jl	`91.66% <89.28%> (-8.34%)`	⬇️
GBIF/src/occurrence.jl	`93.22% <90.90%> (-1.12%)`	⬇️
SimpleSDMLayers/src/lib/coordinateconversion.jl	`94.59% <0.00%> (-5.41%)`	⬇️
...MDatasets/src/providers/CHELSA/chelsa_future_v2.jl	`100.00% <0.00%> (ø)`
...DMDatasets/src/providers/WorldClim/worldclim_v2.jl	`92.10% <0.00%> (ø)`
SimpleSDMDatasets/src/interface.jl	`63.33% <0.00%> (ø)`
...mpleSDMDatasets/src/providers/EarthEnv/earthenv.jl	`86.66% <0.00%> (ø)`
...MDatasets/src/providers/CHELSA/chelsa_future_v1.jl	`100.00% <0.00%> (ø)`
SimpleSDMDatasets/src/keychecker.jl	`74.13% <0.00%> (ø)`
... and 6 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

* 🐛 Stitch returns the wrong bounding box in the end Fixes #154 * 🟩 test the stitch patch * 🟩 some more tests for the right coordinate * 🟩 removed a test where the layer type changes

* 🤢 GBIF throws 404 (not 410) for deleted resources * 🐛 throws an error when the occurrence is not available * ✅ 404 error on single occurrence is handled

tpoisot · 2023-02-18T16:26:26Z

(brought the new commits from main in - this means the GBIF issue with the deleted occurrence in the test is fixed)

tpoisot · 2023-02-18T16:47:03Z

@gabrieldansereau all yours, when you've done your review I will merge and tag

gabrieldansereau

(This comment was meant to be BEFORE my previous commits)

This happens to fix the issue I mentioned in #152 when clipping with non-natural coordinates (with cl1 in the tests). However, it brings back the same issues I fixed in #144 for natural ones, and I suspect it just happens to work with cl1 but could possibly also bring issues with non-natural ones.

julia> M = rand(Bool, (10, 10));
       S = SimpleSDMPredictor(M, 0.0, 1.0, 0.0, 1.0);
       cl1 = clip(S; left = 0.2, right = 0.6, bottom = 0.5, top = 1.0) # all fine
SDM predictor → 5×4 grid with 20 Bool-valued cells
  Latitudes     0.5 ⇢ 1.0
  Longitudes    0.2 ⇢ 0.6

julia> M2 = rand(Bool, (1080, 2160));
       S2 = SimpleSDMPredictor(M2, -180.0, 180.0, -90.0, 90.0);
       exact_lats = (S2.bottom+1.0):1.0:(S2.top-1.0);
       all([isequal(l, clip(S2; top=l).top) for l in exact_lats]) # fails
       show([clip(S2; top=l).top for l in exact_lats])
[-89.0, -88.0, -87.0, -86.0, -85.0, -84.0, -83.0, -82.0, -81.0, -80.0, -79.0, -78.0, -77.0, -76.0, -75.0, -74.0, -73.0, -72.0, -71.0, -70.0, -69.0, -68.0, -67.0, -66.0, -65.0, -64.0, -63.0, -62.0, -61.0, -60.0, -59.0, -58.0, -57.0, -56.0, -55.0, -54.0, -53.0, -52.0, -51.0, -50.0, -49.0, -48.0, -47.0, -46.0, -45.0, -44.0, -43.0, -42.0, -41.0, -40.0, -39.0, -38.0, -36.99999999999999, -35.99999999999999, -35.0, -34.0, -33.0, -31.999999999999996, -30.83333333333333, -29.833333333333332, -29.0, -28.0, -26.83333333333334, -25.999999999999993, -24.999999999999993, -24.0, -23.0, -22.0, -21.0, -19.83333333333333, -18.83333333333333, -18.0, -17.0, -16.0, -14.999999999999993, -13.999999999999993, -13.0, -12.0, -11.0, -10.0, -8.833333333333329, -7.833333333333329, -7.0, -6.0, -5.0, -3.999999999999993, -2.999999999999993, -2.0, -1.0, 0.0, 1.0, 2.0, 3.000000000000014, 4.0, 5.0, 6.0, 7.0, 8.166666666666671, 9.000000000000014, 10.0, 11.0, 12.0, 13.0, 14.166666666666671, 15.0, 16.0, 17.0, 18.0, 19.16666666666667, 20.000000000000014, 21.0, 22.0, 23.0, 24.0, 25.16666666666667, 26.00000000000001, 27.000000000000004, 28.0, 29.0, 30.16666666666667, 31.00000000000001, 32.0, 33.0, 34.0, 35.0, 36.16666666666667, 37.000000000000014, 38.0, 39.0, 40.0, 41.0, 42.0, 43.0, 44.0, 45.0, 46.0, 47.0, 48.0, 49.0, 50.0, 51.0, 52.0, 53.0, 54.0, 55.0, 56.0, 57.0, 58.0, 59.166666666666664, 60.0, 61.0, 62.0, 63.0, 64.0, 65.0, 66.0, 67.0, 68.0, 69.0, 70.0, 71.0, 72.0, 73.0, 74.0, 75.0, 76.0, 77.0, 78.0, 79.0, 80.0, 81.0, 82.0, 83.0, 84.0, 85.0, 86.0, 87.0, 88.0, 89.0]

From the last line I think there are two issues here. The most important one is that clip(S2; top=8.0).top returns 8.166666666666671 as the limit. This means one extra cell is included in the layer, so there would be an overlap if using that coordinate as bottom and top clip.

julia> clip(S2; top=8.0).top
8.166666666666671

julia> clip(S2; top=8.0) # should end at 8.0
SDM predictor → 589×2160 grid with 1272240 Bool-valued cells
  Latitudes     -90.0 ⇢ 8.166666666666671
  Longitudes    -180.0 ⇢ 180.0

julia> clip(S2; bottom=8.0) # should start at 8.0
SDM predictor → 492×2160 grid with 1062720 Bool-valued cells
  Latitudes     7.999999999999986 ⇢ 90.0
  Longitudes    -180.0 ⇢ 180.0

The 2nd issue is just a "display" one. The coordinate is not exactly the one used for clipping because of an approximation difference coming from LinRange, but the grid dimension is correct. Maybe this does not need to be fixed, but I think it would be nice to fix the display.

julia> clip(S2; top=3.0).top
3.000000000000014

gabrieldansereau · 2023-02-21T16:14:42Z

I fixed the issues I described above and added some tests for them. It might not be necessary to test on every coordinate for a layer similar to the WorldClim 10 arcmin one as I'm doing here, but at least it's allowing us to catch all approximation errors.

I tried to clip with gdalwarp to validate my fix (for top=8.0), and now we have exactly the same layer dimensions and coordinates (this would have failed earlier).

julia> l1 = SimpleSDMPredictor(RasterData(WorldClim2, BioClim));
       SpeciesDistributionToolkit.save("l1.tif", l1);
       run(`gdalwarp -te -180.0 -90.0 180.0 8.0 l1.tif l2.tif`);
       l2 = SpeciesDistributionToolkit._read_geotiff("l2.tif", SimpleSDMPredictor);
       l3 = clip(l1; top=8.0);

julia> size(l2) == size(l3)
true

julia> boundingbox(l2) == boundingbox(l3)
true

julia> l2 == l3
true

tpoisot · 2023-02-21T16:48:11Z

If this runs, can you add the test using gdalwarp (using the version that comes with GDAL.jl) to the tests and mark the review as approved?

gabrieldansereau · 2023-02-21T21:08:43Z

@tpoisot I added the code to test with GDAL.jl, but it requires writing and reading a layer, which is handled by SpeciesDistributionToolkit.

I think it's better to leave this as a test for SimpleSDMLayers only, so I commented out the GDAL-related code and tested directly with the layer dimensions that would be returned by GDAL.

tpoisot · 2023-02-21T21:10:07Z

Cool, will merge -- can you open another PR to put the test at the top level of SpeciesDistributionToolkit (there is a folder for tests of edge cases, I think this qualifies)?

🩹 SimpleSDMLayers version bump

b635413

gabrieldansereau and others added 2 commits February 17, 2023 20:46

✅ test for clipping with natural coordinates (#152)

0131c26

🐛 no use of stride in clip / expand keyword

afaf299

tpoisot requested a review from gabrieldansereau February 18, 2023 02:41

tpoisot added 4 commits February 17, 2023 22:07

🐛 Update the stitching code to get the right bounding box (#155)

6730445

* 🐛 Stitch returns the wrong bounding box in the end Fixes #154 * 🟩 test the stitch patch * 🟩 some more tests for the right coordinate * 🟩 removed a test where the layer type changes

➖ drop dependencies for SimpleSDMLayers test

a1f04a8

🟩 tests with the expand keyword

8f45557

GBIF throws 404 (not 410) for deleted resources (#156)

75d8063

* 🤢 GBIF throws 404 (not 410) for deleted resources * 🐛 throws an error when the occurrence is not available * ✅ 404 error on single occurrence is handled

tpoisot changed the title ~~🩹 SimpleSDMLayers version bump~~ 🩹 SimpleSDMLayers handling of clip with coordinates on cell boundaries Feb 18, 2023

🟥 remove the subsetting test (handled by clip)

80e9739

tpoisot marked this pull request as ready for review February 18, 2023 16:40

gabrieldansereau added 3 commits February 21, 2023 10:10

🐛 fix clipping with natural coordinates

25ab9ba

🐛 fix approximation in displayed coordinate

9437554

🐛 tests when clipping exactly on the limit

bf98b78

gabrieldansereau requested changes Feb 21, 2023

View reviewed changes

gabrieldansereau added 2 commits February 21, 2023 16:01

✅ add test comparing clip with gdalwarp

18a4148

🐛 missing end for test module

824bab3

gabrieldansereau approved these changes Feb 21, 2023

View reviewed changes

tpoisot merged commit bbaae1d into main Feb 21, 2023

tpoisot deleted the hotfix/clip branch February 21, 2023 21:11

gabrieldansereau mentioned this pull request Feb 21, 2023

✅ add edge case test for clip in SpeciesDistributionToolkit #159

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🩹 SimpleSDMLayers handling of clip with coordinates on cell boundaries #153

🩹 SimpleSDMLayers handling of clip with coordinates on cell boundaries #153

tpoisot commented Feb 18, 2023 •

edited

Loading

github-actions bot commented Feb 18, 2023

tpoisot commented Feb 18, 2023 •

edited

Loading

codecov-commenter commented Feb 18, 2023 •

edited

Loading

tpoisot commented Feb 18, 2023

tpoisot commented Feb 18, 2023

gabrieldansereau left a comment •

edited

Loading

gabrieldansereau commented Feb 21, 2023 •

edited

Loading

tpoisot commented Feb 21, 2023

gabrieldansereau commented Feb 21, 2023

tpoisot commented Feb 21, 2023 •

edited

Loading

🩹 SimpleSDMLayers handling of clip with coordinates on cell boundaries #153

🩹 SimpleSDMLayers handling of clip with coordinates on cell boundaries #153

Conversation

tpoisot commented Feb 18, 2023 • edited Loading

github-actions bot commented Feb 18, 2023

tpoisot commented Feb 18, 2023 • edited Loading

codecov-commenter commented Feb 18, 2023 • edited Loading

Codecov Report

tpoisot commented Feb 18, 2023

tpoisot commented Feb 18, 2023

gabrieldansereau left a comment • edited Loading

Choose a reason for hiding this comment

gabrieldansereau commented Feb 21, 2023 • edited Loading

tpoisot commented Feb 21, 2023

gabrieldansereau commented Feb 21, 2023

tpoisot commented Feb 21, 2023 • edited Loading

tpoisot commented Feb 18, 2023 •

edited

Loading

tpoisot commented Feb 18, 2023 •

edited

Loading

codecov-commenter commented Feb 18, 2023 •

edited

Loading

gabrieldansereau left a comment •

edited

Loading

gabrieldansereau commented Feb 21, 2023 •

edited

Loading

tpoisot commented Feb 21, 2023 •

edited

Loading