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

Issue with floating nodata on latest rasterio versions #293

Closed
rhugonnet opened this issue Sep 3, 2022 · 4 comments
Closed

Issue with floating nodata on latest rasterio versions #293

rhugonnet opened this issue Sep 3, 2022 · 4 comments
Labels
bug Something isn't working

Comments

@rhugonnet
Copy link
Contributor

The reason our tests kept failing with Longyearbyen example in xDEM, that has a floating nodata of -3.4028235e+38. See for example here.

Locally, the environment solved for rasterio 1.2.10 which passes tests (must have been the same for @adehecq). But on the CI, it solved for 1.3.2 which kept failing! 1.2.10 is the last release before 1.3, so this is where the error might have been introduced. I did not see any issue on it in rasterio yet, it would nice to work out a reproducible example and open an issue there.

@rhugonnet rhugonnet added the bug Something isn't working label Sep 3, 2022
@rhugonnet
Copy link
Contributor Author

Update: this problem might not actually be rasterio, and is still not solved... 😢
The xDEM PRs for which tests passed yesterday and were merged now fail when rerun (see this one with two CI runs, one failing, one succeeding): https://github.com/GlacioHack/xdem/actions/runs/2985261002. I compared the environments built by mamba for the two runs, and they are exactly the same. Same with other PRs, that now fail with the same nodata error.

My guess right now: this may be related to floating point precision.
I also thought about a random number generation that is not fixed within xDEM's coreg.py, and would pollute the reprojection/nodata downstream. But we would have seen it earlier... Additionally, NumPy random generation is fixed cross-platform, and we are using the legacy RandomState or np.random.seed that is also fixed across NumPy versions: https://stackoverflow.com/questions/40676205/cross-platform-numpy-random-seed, so it should not be possible.

Looking into it...

@rhugonnet
Copy link
Contributor Author

The above problem was actually a combination of an issue now described in #294, and the behaviour of >1.2.10 rasterio.

But reverting the environment to rasterio >=1.3 in xDEM's CI showed that there is indeed an issue with rasterio's handling of nodata of floating precision starting in 1.3. It is easy to reproduce as it makes our xDEM test_examples.py fail.

@rhugonnet
Copy link
Contributor Author

Still an issue for floating nodata, no way to merge GlacioHack/xdem#301 with rasterio>=1.3.

But forcing <=1.2.10 in xDEM is in direct conflict with the forcing of >1.3 on the latest GeoUtils for the cropping bugs... we'll have to find a solution soon!

@rhugonnet
Copy link
Contributor Author

rhugonnet commented Sep 17, 2022

Alelujah, finally found the issue, after 2 weeks... 😓. Nothing to do with floating nodatas ! It actually comes from pytest-xdist: running some tests in parallel lead to I/O issues. When reading, randomly, all grid points could be considered as nodata, which started recently.

This is likely because we added more tests that rely on the ddem + also on the error .tif that is written in test_spatialstats.py. To solve the issue while preserving pytest-xdist running in parallel, we'd have to find a way to force pytest to first write all files to disk without parallelization, and then perform the rest of the tests. A bit like it was done here but more robust so it does not break when introducing a new test: https://github.com/GlacioHack/xdem/blob/main/.github/workflows/python-package.yml#L53

But overall, pytest-xdist does not improve the test speed so much, so the solution is easy in the meantime: GlacioHack/xdem@0a59b42.

I will add an article to the xDEM wiki.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant