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

Use Priority Flood algorithm to fill depressions #243

Merged
merged 4 commits into from
Feb 19, 2024

Conversation

groutr
Copy link
Contributor

@groutr groutr commented Feb 5, 2024

Fixes #239

Runtime is about 7.5min on a (12150, 9622) float32 raster which is acceptable.

This variant of the algorithm (as presented in the paper linked in the issue) uses a plain queue to avoid some of the overhead of using the priority queue. Since numba doesn't have a plain queue, I use a list that gets flushed periodically.
A critical part of making this fast was using numba's typedlist which still seems to be experimental according to the docs.

I'm open to feedback.

@groutr
Copy link
Contributor Author

groutr commented Feb 7, 2024

@mdbartos I've implemented most of the variants of the priority-flood algorithm described in this paper into a priority-flood module on my machine. There are methods for finding the basins, filling depressions, computing flow direction, and resolving flats (though I had to slightly deviate from the algorithm in the paper to make in work for test DEM). Do you want these other methods in pysheds too?

pysheds/sgrid.py Outdated Show resolved Hide resolved
The decorator hides an implementation detail of numba that the caller need not worry about.
@groutr
Copy link
Contributor Author

groutr commented Feb 14, 2024

@mdbartos I don't think the nodata_out parameter is needed any longer in fill_depressions. How do you feel about deprecating it?

@mdbartos
Copy link
Owner

  • Sure, I'm fine with deprecating it.
  • I'm interested in the other methods, but I will pull in this PR first. Feel free to submit another PR with these methods included if you think there's value.

Thanks again and awesome work,
MDB

@mdbartos mdbartos merged commit 56e6aea into mdbartos:master Feb 19, 2024
5 checks passed
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.

Preprocessing DEM with pysheds seems to produce incorrect accumulations
2 participants