Skip to content
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

Handle non-deterministic pyproj failure #2259

Merged
merged 2 commits into from
Oct 10, 2024
Merged

Conversation

AdeelH
Copy link
Collaborator

@AdeelH AdeelH commented Oct 10, 2024

Overview

This PR fixes a problem reported in #2257 and #2243 where RasterioSource.get_chip() would result in the error below:

  File "/opt/src/rastervision_core/rastervision/core/data/label_source/semantic_segmentation_label_source.py", line 79, in get_label_arr
    label_arr = self.raster_source.get_chip(window)
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/src/rastervision_core/rastervision/core/data/raster_source/rasterio_source.py", line 172, in get_chip
    chip = self._get_chip(window, out_shape=out_shape, bands=bands)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/src/rastervision_core/rastervision/core/data/raster_source/rasterio_source.py", line 147, in _get_chip
    chip = fill_overflow(self.bbox, window, chip)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/src/rastervision_core/rastervision/core/data/utils/raster.py", line 35, in fill_overflow
    chip[..., :, xmax:, :] = fill_value
    ~~~~^^^^^^^^^^^^^^^^^^
TypeError: slice indices must be integers or None or have an __index__ method

This was tracked down to be the result of pyproj.Transformer.transform() non-deterministically returning infs in some cases ... apparently due to network connectivity issues! More discussion:

This PR also adds some validation code in the Box constructor to disallow non-finite values. This will allow similar errors to be caught earlier in the future.

Checklist

  • Added unit tests, if applicable
  • Updated documentation, if applicable
  • Added needs-backport label if the change should be back-ported to the previous release
  • PR has a name that won't get you publicly shamed for vagueness

Notes

N/A

Testing Instructions


Closes #2257

For some transformations, pyproj attempts to download transformation
grids from the internet for improved accuracy when
Transformer.transform() is called. If it fails to connect to the
internet, it silently returns (inf, inf) and silently modifies its
behavior to not access the internet on subsequent calls, causing
them to succeed (though possibly with a loss of accuracy). See
pyproj4/pyproj#705 for details.

This workaround forces an error to be raised by setting
errcheck=True and ignoring the first error.
@AdeelH AdeelH added the needs-backport This PR needs to be backported to release branches label Oct 10, 2024
@AdeelH AdeelH marked this pull request as ready for review October 10, 2024 19:01
@AdeelH AdeelH merged commit 59149f8 into azavea:master Oct 10, 2024
2 checks passed
Copy link

codecov bot commented Oct 10, 2024

Codecov Report

Attention: Patch coverage is 50.00000% with 10 lines in your changes missing coverage. Please review.

Project coverage is 90.31%. Comparing base (10a9c98) to head (59e65e2).
Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
...e/data/crs_transformer/rasterio_crs_transformer.py 44.44% 10 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2259      +/-   ##
==========================================
- Coverage   90.40%   90.31%   -0.09%     
==========================================
  Files         199      199              
  Lines        9908     9925      +17     
==========================================
+ Hits         8957     8964       +7     
- Misses        951      961      +10     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@AdeelH AdeelH deleted the proj-bug branch October 16, 2024 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-backport This PR needs to be backported to release branches
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RasterioSource.get_chip() raises TypeError: slice indices must be integers or None or have an __index__ method
1 participant