-
Notifications
You must be signed in to change notification settings - Fork 96
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix bug that would add spurious nan lines in gradient search #643
Conversation
Fixed images from the use case in #642: |
I restarted CI, there was a hickup with Conda. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Simple fix, but must've been a pain to hunting the bug 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good from what I understand. I had one question just to clear something up for me, but otherwise good to merge.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #643 +/- ##
=======================================
Coverage 93.95% 93.96%
=======================================
Files 86 86
Lines 13541 13546 +5
=======================================
+ Hits 12723 12728 +5
Misses 818 818
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
This PR fixes #642.
The symptom was that every other line, or all lines, of chunks that were on the edges of a geos disc would be nans. That was because the cursor in the gradient search algorithm was not moved along x when a pixel was skipped because of invalid target coordinates.