watershed-workflow
watershed-workflow copied to clipboard
New densification methods, variable quad length-scales. increased flexibility for defining quad widths, depths
This pull request provides improvements to the stream-alined mixed-polyhedral meshing. The main changes include:
- Variable quad lengths: The user can provide different target quad lengths for different reaches. This helps with improving cell aspect ratios for thinner lower-order streams.
- Widths and Supplemental Depths: Now user can use arbitrary conditions to provide quad widths and supplemental depths. Previously, only dictionaries with stream order as keys or drainage area-based functions were accepted.
- New densification: Leveraging shapely.LineString-based interpolations while preserving endpoints, the new method yields much more uniform quad lengths compared to the previous method. Currently, the new method is available as
densify_rivers_new
, hopefully in a couple of months, once tested on a range of cases, will become the default. Then we can support the old method asdensify_rivers_old
for reproducibility.
There are some updates in progress for the convexity-enforcing function to make it more robust. Once, that is done, we can go ahead with this PR.
all tests are passing on the local machine. The failing test is due to NHDPlus dataset name change
cleaned up code as suggested, and checked all examples. Tests are passing on the local machine. CI is failing at NHD manager due to a property name change. As suggested by Chucho, we can add the patch below, for now; we anyway map all kinds of IDs to "ID" in the properties later in the workflow. Maybe we can bring that mapping at the outset in the source manager itself?
In manager_nhd.py after line 254
if ('NHDPlusID' not in reaches[0]['properties'].keys()) & ('nhdplusid' in reaches[0]['properties'].keys()):
for reach in reaches:
reach['properties']['NHDPlusID'] = reach['properties'].pop('nhdplusid')
cleaned up code as suggested, and checked all examples. Tests are passing on the local machine. CI is failing at NHD manager due to a property name change. As suggested by Chucho, we can add the patch below, for now; we anyway map all kinds of IDs to "ID" in the properties later in the workflow. Maybe we can bring that mapping at the outset in the source manager itself?
In manager_nhd.py after line 254
if ('NHDPlusID' not in reaches[0]['properties'].keys()) & ('nhdplusid' in reaches[0]['properties'].keys()): for reach in reaches: reach['properties']['NHDPlusID'] = reach['properties'].pop('nhdplusid')
That's fine by me as a temporary fix. Please use "and" over "&" -- they aren't the same thing (one is integer math, the other is boolean comparison operator).