DOI-USGS / hyRefactor

https://code.usgs.gov/wma/nhgf/reference-fabric/hyrefactor
Creative Commons Zero v1.0 Universal
5 stars 0 forks source link

Move NGEN hyAggregate workflow over? #32

Open mikejohnson51 opened 2 years ago

mikejohnson51 commented 2 years ago

I would like to move the aggregation workflow from hyAggregate over here if you agree.

This will be a large PR so wanted to check first.

dblodgett-usgs commented 2 years ago

Yeah -- let's do it. But as we do, I want to make sure we are being diligent about test coverage.

I need to prioritize #25 and get it merged. Can we plan on you working on a PR after I've had time to do that?

mikejohnson51 commented 2 years ago

yep! Ill watch that.

dblodgett-usgs commented 2 years ago

As we discussed -- there's a chance that our two methods converge around this function: https://github.com/dblodgett-usgs/hyRefactor/blob/main/R/aggregate_network.R#L77

If, instead of "make_outlets_valid" we were to call a function that returned the required distribution of nexuses (outlets).

From our chat:

The problem with the distribution approach is that it requires iterative aggregations so by the time the outlets are identified the topology/geometry's are already fully aggregated

Yes -- so don't use https://github.com/dblodgett-usgs/hyRefactor/blob/main/R/aggregate_network.R#L82.

The key is that you create catchment sets and flowpath sets that get aggregated into a consistent data structure.