Fix using cached LUTs in bilinear resampler
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
- [x] Passes
git diff origin/main **/*py | flake8 --diff
Codecov Report
Merging #438 (1f9f20d) into main (74eb088) will increase coverage by
0.07%. The diff coverage is96.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%) |
:arrow_up: |
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%) |
:arrow_down: |
| pyresample/bilinear/xarr.py | 92.90% <100.00%> (ø) |
|
| pyresample/spherical.py | 98.21% <100.00%> (+0.01%) |
:arrow_up: |
| 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%) |
:arrow_up: |
| pyresample/_spatial_mp.py | 82.94% <0.00%> (-0.63%) |
:arrow_down: |
| pyresample/geometry.py | 87.17% <0.00%> (-0.11%) |
:arrow_down: |
| 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.
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.
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.
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:
- You are accidentally running a different version locally than you think you are (not
mainwhich should break). - 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.
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.
I have to finish my docstring updates for my antimeridian PR but then I'll switch branches and test on main.
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.
I'm starting my holiday now, so I'll get back to this in August at the earliest.
Ok, I've tracked down why the real world case fails and the test doesn't. It is this line of code:
https://github.com/pytroll/pyresample/blob/e531bd9c8bc42213ee128a09842797a1346dee06/pyresample/bilinear/xarr.py#L113
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.
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.
Damn, had forgotten this. I'll see if I have time early next week to have a look.
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:
https://github.com/pytroll/pyresample/blob/e531bd9c8bc42213ee128a09842797a1346dee06/pyresample/bilinear/xarr.py#L113
Same with a partially overlapping target area (added with the latest commit to this PR) that gets a sample using pre-calculated LUTs.
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.
Coverage increased (+0.005%) to 94.13% when pulling 1f9f20d4b5366b123c45755e4ad65aa906df6019 on pnuu:bugfix-bilinear-cache-loading into df76244fded7011ef09269b921c6fee981319a92 on pytroll:main.
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'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.