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

Tests failing on main #1562

Closed
emlys opened this issue Apr 26, 2024 · 2 comments · Fixed by #1563
Closed

Tests failing on main #1562

emlys opened this issue Apr 26, 2024 · 2 comments · Fixed by #1563
Labels
bug Something isn't working critical for issues likely to obstruct the whole dev team (e.g. broken builds)

Comments

@emlys
Copy link
Member

emlys commented Apr 26, 2024

Tests are failing on macOS, all python versions. Probably due to some dependency update.

FAILED tests/test_coastal_vulnerability.py::CoastalVulnerabilityTests::test_aoi_invalid_geometry - RuntimeWarning: invalid value encountered in intersection
FAILED tests/test_ndr.py::NDRTests::test_masking_invalid_geometry - AssertionError: 
Arrays are not equal

Mismatched elements: 1 / 1 (100%)
Max absolute difference: 1
Max relative difference: inf
 x: array([[1]])
 y: array([[0]], dtype=uint8)
FAILED tests/test_scenic_quality.py::ViewshedTests::test_primitive_peak - AssertionError: 
Arrays are not equal

Mismatched elements: 4 / 64 (6.25%)
Max absolute difference: 1.
Max relative difference: 1.
 x: array([[1, 1, 1, 1, 1, 1, 1, 1],
       [1, 1, 1, 1, 1, 1, 1, 1],
       [1, 1, 1, 1, 1, 1, 1, 1],...
 y: array([[1., 1., 1., 1., 1., 1., 1., 1.],
       [1., 1., 1., 1., 1., 1., 1., 1.],
       [1., 1., 1., 1., 1., 1., 1., 1.],...
@emlys emlys added bug Something isn't working critical for issues likely to obstruct the whole dev team (e.g. broken builds) labels Apr 26, 2024
@emlys
Copy link
Member Author

emlys commented Apr 29, 2024

This appears to be related to a change in the github actions runner. macos-latest is now macos-14-arm64. Since the difference has to do with ARM64 and/or ARM64 versions of packages, I can't reproduce it locally.

@emlys
Copy link
Member Author

emlys commented Apr 30, 2024

Coastal Vulnerability test failure

This was failing due to a RuntimeWarning coming from shapely. This looks like a M1 specific bug with shapely or geos? I asked about it here: shapely/shapely#2045
We can add an exception to our pytest configuration so that this specific warning doesn't fail the test.

Scenic Quality test failure

@phargogh was able to reproduce this one locally on his M1 mac. It comes down to a difference in floating point error between Intel and M1. The relevant code is:

if (adjusted_dem_height >= z and
target_distance < max_visible_radius and
target_dem_height != nodata):
visibility_managed_raster.set(ix_target, iy_target, 1)
aux_managed_raster.set(ix_target, iy_target, adjusted_dem_height)
else:
visibility_managed_raster.set(ix_target, iy_target, 0)
aux_managed_raster.set(ix_target, iy_target, z)

We have a situation where adjusted_dem_height and z should be equal, but on M1, z has floating point error that makes it greater than adjusted_dem_height, and so the conditional evaluates differently. This is like how need isclose for equality comparison, we need a similar thing for greater/lesser-than-or-equal comparison. Probably worth scanning the rest of our code base for this situation.

NDR test failure

The failing test uses an invalid geometry, which is questionable, but I reproduced the issue with a valid geometry too. The issue comes down to different results from gdal.RasterizeLayer (reported here: OSGeo/gdal#9822). Ultimately, seems to be an upstream issue and we don't need to specifically test this edge case. We could test with an invalid geometry that more obviously overlaps the centerpoint, or remove this test case altogether - we don't need to support invalid geometries anyway.

phargogh added a commit to phargogh/invest that referenced this issue Apr 30, 2024
phargogh added a commit to phargogh/invest that referenced this issue Apr 30, 2024
phargogh added a commit to phargogh/invest that referenced this issue Apr 30, 2024
emlys added a commit to emlys/invest that referenced this issue May 1, 2024
The old geometry had its self intersection on the pixel centerpoint,
which should work, but didn't on M1 mac runners (see natcap#1562).
This modifies the geometry so that the pixel centerpoint is fully
within the geometry, which I'm hoping is more stable.
phargogh added a commit to phargogh/invest that referenced this issue May 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working critical for issues likely to obstruct the whole dev team (e.g. broken builds)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant