D8 routing #1633
D8 routing #1633
Conversation
| @@ -337,15 +340,15 @@ class ManagedRaster { | |||
| } | |||
| }; | |||
|
|
|||
| template<class T> | |||
There was a problem hiding this comment.
This is a class template. ManagedFlowDirRaster is not a class, but ManagedFlowDirRasater<MFD> and ManagedFlowDirRasater<D8> are classes that can be created from this template.
| ManagedFlowDirRaster() {} | ||
|
|
||
| ManagedFlowDirRaster(char* raster_path, int band_id, bool write_mode) | ||
| : ManagedRaster(raster_path, band_id, write_mode) {} | ||
|
|
||
| template<typename T_ = T, std::enable_if_t<std::is_same<T_, MFD>::value>* = nullptr> |
There was a problem hiding this comment.
This tells the compiler to use this implementation of is_local_high_point if the template argument is MFD. The same pattern is used in all the iterator classes below.
There was a problem hiding this comment.
Interesting! So then if we were to extend these classes to accommodate some additional routing method, we would need to add new classmethods that have a conditional like this?
There was a problem hiding this comment.
I really don't have anything to suggest at this time ... the solution's architecture makes sense to me given the constraints of a compiled language, I'm not seeing any compiler warnings, tests pass, and the outputs all make sense to me, as do the tests. This looks great!
| ManagedFlowDirRaster() {} | ||
|
|
||
| ManagedFlowDirRaster(char* raster_path, int band_id, bool write_mode) | ||
| : ManagedRaster(raster_path, band_id, write_mode) {} | ||
|
|
||
| template<typename T_ = T, std::enable_if_t<std::is_same<T_, MFD>::value>* = nullptr> |
There was a problem hiding this comment.
Interesting! So then if we were to extend these classes to accommodate some additional routing method, we would need to add new classmethods that have a conditional like this?
| if algorithm == 'MFD': | ||
| run_effective_retention[MFD](*args) | ||
| else: # D8 | ||
| run_effective_retention[D8](*args) |
There was a problem hiding this comment.
I get it now! That is really pretty neat. So the implication is that the compiler is, effectively, rendering MFD and D8 versions of the underlying functions to allow us to call either one at runtime. Awesome.
Description
This PR implements D8 equivalents of the routing functions that previously only supported MFD.
A "routing algorithm" option input (MFD or D8) is added to each model. Based on that option, the appropriate pygeoprocessing routing functions are called. For the model-specific routing functions, I used C++ templates to implement switching between MFD and D8 "modes" on the iterator classes in ManagedRaster.h. To make it possible to call the templated functions, I rewrote the bulk of the SDR, NDR, and SWY routing functions from Cython to C++. I'm happy to discuss pros and cons of this.
Future work: add logging to python from C++ extensions, add templating for
ManagedRasterto read and return values of different data typesChecklist