Skip to content

generate_terrain: cupy case, dask numpy case #555

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

Merged
merged 16 commits into from
Sep 28, 2021
Merged

generate_terrain: cupy case, dask numpy case #555

merged 16 commits into from
Sep 28, 2021

Conversation

thuydotm
Copy link
Contributor

No description provided.

@thuydotm
Copy link
Contributor Author

@kiliakis I've refactored the code and added some tests. It happened that the results when running with vanilla numpy, dask numpy, and cupy backed DataArrays are not the same. Similar things happened when running with the original code you added. Can you investigate and make sure the results match? I can help to take a look into the dask case.

@thuydotm thuydotm mentioned this pull request Sep 17, 2021
@kiliakis
Copy link
Contributor

@thuydotm The perlin noise function (that is also used by generate terrain) uses a random number generator (see here for example: https://github.com/makepath/xarray-spatial/pull/555/files?file-filters%5B%5D=.md&file-filters%5B%5D=.py#diff-4599a7f9c05a7872e774fd7d4fa503515a3e5eecbed9d36ed379bb2266491a62R71).
The numpy implementation uses numpy.random and Cupy uses cupy.random. These functions do no return the same sequence of random numbers.
So my guess is this is why we don't see the same results. To see the same results, we will have to modify the code to not shuffle the p array (see here: https://github.com/makepath/xarray-spatial/pull/555/files?file-filters%5B%5D=.md&file-filters%5B%5D=.py#diff-4599a7f9c05a7872e774fd7d4fa503515a3e5eecbed9d36ed379bb2266491a62R76). But this is something that can be used for testing only.

@thuydotm
Copy link
Contributor Author

@kiliakis I merged your PR and made code changes to ensure the same results when running with numpy, dask and cupy. Going to merge this PR to main branch. Please feel free to open new PRs if you see anything to improve.

@thuydotm thuydotm merged commit b236393 into master Sep 28, 2021
@giancastro giancastro mentioned this pull request Oct 6, 2021
@thuydotm thuydotm deleted the pr/552 branch December 23, 2021 06:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants