Minor bug fix within terrain correction#121
Conversation
Minor bug fix within terrain correction that caused a DEM query error under an extreme pointing scenario (intersect ellipsoid with near 90deg off nadir pointing).
|
Looks good! |
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project check has failed because the head coverage (81.30%) is below the target coverage (95.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #121 +/- ##
=======================================
Coverage 73.62% 73.62%
=======================================
Files 67 67
Lines 10655 10656 +1
Branches 1204 1204
=======================================
+ Hits 7845 7846 +1
Misses 2343 2343
Partials 467 467 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| gd_cur_alt[idx_cont] = gd_cur_pos_subset[:, 2] | ||
|
|
||
| # Look up the ellipsoidal height (surface) for this position. | ||
| gd_prev_h = gd_cur_h |
There was a problem hiding this comment.
Only question is if you want to add this separate change to this PR:
gd_prev_h = gd_cur_h.copy()
mmaclay
left a comment
There was a problem hiding this comment.
Looks good to go. There some subtle semantics changes as we now expect gd_cur_h will have NaNs from the beginning, but assuming the testing looks good, this should be an improvement overall.
I added a comment about changing line 941 only if you want to include it in this PR. I think we probably want to reference a copy not the original variable there, but feel free to leave for a later discussion.
|
Just updated this branch w/ main to keep this PR from timeout closing automatically. |
Minor bug fix within terrain correction that caused a DEM query error under an extreme pointing scenario (intersect ellipsoid with near 90deg off nadir pointing). Required for Clarreo end-to-end testing.
This same logic (excluding high off nadir points from DEM query) occurs later within the terrain correction. The bug was that it wasn't being excluded from the initial query. We never encountered an issue before because usually (every time before) the first query is within the bounding box of the queried DEM data.