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

Move NGEN hyAggregate workflow over? #32

Open
mikejohnson51 opened this issue Feb 3, 2022 · 3 comments
Open

Move NGEN hyAggregate workflow over? #32

mikejohnson51 opened this issue Feb 3, 2022 · 3 comments

Comments

@mikejohnson51
Copy link

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
Copy link
Collaborator

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
Copy link
Author

yep! Ill watch that.

@dblodgett-usgs
Copy link
Collaborator

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.

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

No branches or pull requests

2 participants