Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #708 +/- ##
=======================================
Coverage 93.67% 93.67%
=======================================
Files 89 89
Lines 13621 13652 +31
=======================================
+ Hits 12759 12789 +30
- Misses 862 863 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
djhoese
left a comment
There was a problem hiding this comment.
Nice job. Thanks for finding this. I've always disliked the indexing done in the query so I'm glad to see it cleaned up. I had a couple questions, but otherwise the majority of this looks good to me.
Otherwise could you check pre-commit's failures? Thanks.
| if np.isnan(src_res): | ||
| radius_of_influence = dst_res | ||
| elif np.isnan(dst_res): | ||
| radius_of_influence = src_res | ||
| else: | ||
| radius_of_influence = max(src_res, dst_res) |
There was a problem hiding this comment.
Could you explain why this is better? Because it prefers the source resolution?
There was a problem hiding this comment.
It works just the same and avoids the all-nans warning print, which is quite expensive in Python. So it benefits only the worst-case scenario.
a17a10b to
2d09c3b
Compare
|
In your original description you talked about the performance of |
|
Oh and what was your test case for your timing? I'm surprised by how fast the query stuff was, but even more by how slow the NaN case was for the radius of influence stuff. |
|
One last thing: I'm not ignoring your other pyresample and satpy PRs, but I'm still catching up on real work after being out for a couple days and I'm trying to fully understand the changes before commenting. Thanks again for all these changes/fixes/improvements. |
query_no_distance:
runtime:
0.01383s -> 0.01279s(-7.5%).rss:
221096KB -> 204204KB(-7.6%)._verify_input_object_type numpy array:
runtime:
0.02705s -> 0.02002s(-26%).rss: same
_compute_radius_of_influence nan case:
runtime:
19.71s -> 2.10s.rss: same