Fix using cached LUTs in bilinear resampler

[pnuu]

This PR fixes the bilinear resampling when the resampling LUTs have been cached.

The bug were introduced in #324 where a check was added to make sure the data array dimensions are in correct order. The test was done against an index array that only exists during the initial run, and a better comparison is against the shape of the source geo definition.

• Tests added
• Tests passed
• Passes git diff origin/main **/*py | flake8 --diff

[codecov]

Codecov Report

Merging #438 (1f9f20d) into main (74eb088) will increase coverage by 0.07%.
The diff coverage is 96.42%.

@@            Coverage Diff             @@
##             main     #438      +/-   ##
==========================================
+ Coverage   94.20%   94.28%   +0.07%     
==========================================
  Files          68       69       +1     
  Lines       12255    12388     +133     
==========================================
+ Hits        11545    11680     +135     
+ Misses        710      708       -2     

Flag	Coverage Δ
unittests	94.28% <96.42%> (+0.07%)	⬆️

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

Impacted Files	Coverage Δ
pyresample/ewa/ewa.py	75.86% <66.66%> (-0.93%)	⬇️
pyresample/bilinear/xarr.py	92.90% <100.00%> (ø)
pyresample/spherical.py	98.21% <100.00%> (+0.01%)	⬆️
pyresample/spherical_geometry.py	93.72% <100.00%> (ø)
pyresample/test/test_bilinear.py	100.00% <100.00%> (ø)
pyresample/test/test_geometry.py	99.51% <100.00%> (+0.02%)	⬆️
pyresample/_spatial_mp.py	82.94% <0.00%> (-0.63%)	⬇️
pyresample/geometry.py	87.17% <0.00%> (-0.11%)	⬇️
pyresample/geo_filter.py	100.00% <0.00%> (ø)
pyresample/utils/proj4.py	100.00% <0.00%> (ø)
... and 14 more

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

[pnuu]

In addition to Coveralls acting up, I can't get the tests to fail. I tried adding

_ = new_resampler.resample(self.data1)

to https://github.com/pytroll/pyresample/blob/main/pyresample/test/test_bilinear.py#L1198

As far as I understand, this test pretty much does what Satpy is doing, so it should be failing, but passes with the current main.

[pnuu]

Also res = new_resampler.get_sample_from_bil_info(self.data1) works. This is what Satpy is actually doing, while .resample() calls the .get_bil_info() method first.

[djhoese]

Yeah I would have expected adding get_sample_from_bil_info to the line in the test you specified to fail. My best guesses are:

1. You are accidentally running a different version locally than you think you are (not main which should break).
2. Something in satpy and/or the tests and/or the resampler itself is setting some attribute on the class itself so it "persists" when you make the new resampler in the test.

[pnuu]

I'm fairly certain it's the correct version because my import ipdb; ipdb.set_trace() in the test method is caught.

I modified the test so that it saves to a persistent location, then ran again with modification that skipped the saving. Still works fine.

[djhoese]

I have to finish my docstring updates for my antimeridian PR but then I'll switch branches and test on main.

[djhoese]

I also just noticed that I ran Ernst's tests again my antimeridian branch which is relatively old so it may not include things you may have fixed in your more recent PRs.

[pnuu]

I'm starting my holiday now, so I'll get back to this in August at the earliest.

[djhoese]

Ok, I've tracked down why the real world case fails and the test doesn't. It is this line of code:

pyresample/pyresample/bilinear/xarr.py

Line 113 in e531bd9

if res.size != dim_multiplier * self._target_geo_def.size:

From what I understand this if block is only entered if there aren't neighbors for every output pixel (the source data doesn't fully cover the target area). In the test the target def being used is only 4x4 pixels and so res.size is equal to the size of the target area (16). So I think the tests need to be expanded for other coverages. I saw that there is another test area that doesn't overlap the input data, but I'm not sure that is "good enough". I'll try a quick test.

[djhoese]

Hhhmmm even the outside target area doesn't hit this condition. I'm not sure if it is a smart idea for me to dive further into this until you're back @pnuu. Let me know if you have ideas and I can try them out before then.

[pnuu]

Damn, had forgotten this. I'll see if I have time early next week to have a look.

[pnuu]

I tested with the target area that doesn't overlap the input area at all with the main branch code, and it works. And also enters this branch:

pyresample/pyresample/bilinear/xarr.py

Line 113 in e531bd9

if res.size != dim_multiplier * self._target_geo_def.size:

Same with a partially overlapping target area (added with the latest commit to this PR) that gets a sample using pre-calculated LUTs.

[djhoese]

Looks like the test failure is an HTTP hiccup in the deploy test (no big deal) and a new failure in the EWA resampling. I guess I know what I'm working on this morning.

[coveralls]

Coverage increased (+0.005%) to 94.13% when pulling 1f9f20d on pnuu:bugfix-bilinear-cache-loading into df76244 on pytroll:main.

[djhoese]

I'm a little scared about the EWA failure. I didn't change anything and it seems to pass now. I guess I'll deal with it next time...

[djhoese]

LGTM. Feel free to merge.

[pnuu]

I'm a little scared about the EWA failure. I didn't change anything and it seems to pass now. I guess I'll deal with it next time...

I was a bit surprised about the failure in the first place, as only thing I changed was the flake8 complaint. But yeah, merging.