-
Notifications
You must be signed in to change notification settings - Fork 5
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
Remove placeholder edges from the flow network #167
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This change renames `flow_routing_targets` to `flow_routing_d8_targets` and fills the source and target arrays only with valid edges whose source and target nodes are both in the grid. Sinks, outlets and invalid nodes no longer get an outgoing edge pointing to the fictitious -1 node. `flow_routing_d8_carve` now returns a topologically sorted array of /nodes/ and a grid of flow directions. This behaves identically to how it did before, but some things have been renamed and comments updated to reflect this new interpretation of the output. There is a node for every pixel in the grid, and they are sorted according to their position in the topological order of the flow network. `flow_routing_d8_targets` appends edges to the source and target arrays. These must still be as large as the DEM, because the number of edges is unknown, but `flow_routing_d8_targets` returns the number of edges, and the source and target arrays can be resized in the host language if needed. An alternative would be to enumerate edges during the flow routing and return an edge count from `flow_routing_d8_carve`. Then one could allocate source and target arrays of the correct size. Because `flow_routing_d8_targets` now returns only edges that are within the flow network, we do not need to check that the target node is greater than zero in `flow_accumulation` and `drainagebasins`. The tests in `flow_accumulation` to ensure that src and tgt are valid are not required /assuming the source and target arrays have been properly validated/. It is the caller's responsibility to provide valid edge lists. test/random_dem.cpp required significant refactoring to account for this change in the API. Many of the test functions directly manipulate the edge lists, and those had to be modified.: `streamnetwork` generates the stream edge list from the flow direction and accumulation data. It now scans the topologically sorted list of nodes to find nodes whose flow accumulation is greater than the threshold. Then it scan through the flow network edges to find edges that are part of the stream network and appends them to a list. We also no longer have to reverse the list of stream edges, so we can get rid of the intermediate `stream_edges` array. The check in `test_drainagebasins` that the downstream target of each edge is greater than or equal to zero is turned into an assertion, because we should not have any of these placeholder edges remaining in source and target. `test_stream_distance` required minor changes such as the addition of the `edge_count` argument. It should work for either a flow network or a stream network, as long as the appropriate `edge_count` is passed. We use the flow network in the test, so it computes the upstream distance for every pixel in the DEM and then compares it to the upstream distance computed by streamquad_trapz only for pixels in the stream network. Signed-off-by: William Kearney <[email protected]>
wkearn
added a commit
to wkearn/libtopotoolbox
that referenced
this pull request
Feb 10, 2025
With the new flow routing edge lists (TopoToolbox#167), we can no longer guarantee that `source` and `target` are of length `dims[0]*dims[1]`, and we to know need the explicit number of edges in the edge list. Signed-off-by: William Kearney <[email protected]>
wkearn
added a commit
that referenced
this pull request
Feb 10, 2025
With the new flow routing edge lists (#167), we can no longer guarantee that `source` and `target` are of length `dims[0]*dims[1]`, and we to know need the explicit number of edges in the edge list. Signed-off-by: William Kearney <[email protected]>
wkearn
added a commit
to wkearn/pytopotoolbox
that referenced
this pull request
Feb 10, 2025
This commit contains TopoToolbox/libtopotoolbox#167, which significantly changes the flow routing API. pytopotoolbox now fails to build because of these API changes.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves #166
This change renames
flow_routing_targets
toflow_routing_d8_targets
and fills the source and target arrays only with valid edges whose source and target nodes are both in the grid. Sinks, outlets and invalid nodes no longer get an outgoing edge pointing to the fictitious -1 node.flow_routing_d8_carve
now returns a topologically sorted array of /nodes/ and a grid of flow directions. This behaves identically to how it did before, but some things have been renamed and comments updated to reflect this new interpretation of the output. There is a node for every pixel in the grid, and they are sorted according to their position in the topological order of the flow network.flow_routing_d8_targets
appends edges to the source and target arrays. These must still be as large as the DEM, because the number of edges is unknown, butflow_routing_d8_targets
returns the number of edges, and the source and target arrays can be resized in the host language if needed. An alternative would be to enumerate edges during the flow routing and return an edge count fromflow_routing_d8_carve
. Then one could allocate source and target arrays of the correct size.Because
flow_routing_d8_targets
now returns only edges that are within the flow network, we do not need to check that the target node is greater than zero inflow_accumulation
anddrainagebasins
. The tests inflow_accumulation
to ensure that src and tgt are valid are not required /assuming the source and target arrays have been properly validated/. It is the caller's responsibility to provide valid edge lists.test/random_dem.cpp required significant refactoring to account for this change in the API. Many of the test functions directly manipulate the edge lists, and those had to be modified.:
streamnetwork
generates the stream edge list from the flow direction and accumulation data. It now scans the topologically sorted list of nodes to find nodes whose flow accumulation is greater than the threshold. Then it scan through the flow network edges to find edges that are part of the stream network and appends them to a list. We also no longer have to reverse the list of stream edges, so we can get rid of the intermediatestream_edges
array.The check in
test_drainagebasins
that the downstream target of each edge is greater than or equal to zero is turned into an assertion, because we should not have any of these placeholder edges remaining in source and target.test_stream_distance
required minor changes such as the addition of theedge_count
argument. It should work for either a flow network or a stream network, as long as the appropriateedge_count
is passed. We use the flow network in the test, so it computes the upstream distance for every pixel in the DEM and then compares it to the upstream distance computed by streamquad_trapz only for pixels in the stream network.