search

NOAA-OWP / ngen

Next Generation Water Modeling Engine and Framework Prototype
Other
84 stars 63 forks source link

The catchment/waterbody/COMID mapping and relationships need cleanup #89

Open hellkite500 opened 4 years ago

hellkite500 commented 4 years ago

https://github.com/NOAA-OWP/ngen/blob/874110b041036118a0811b270de25a5dbcb17d6c/src/NGen.cpp#L152

Here is an example where a waterbody ID is being treated as a catchment id in order to associate a COMID with a catchment to read its GIUH data. This is technically the wrong semantics, and should not propagate further.

One potential solution is to add the catchment id to the crosswalk.json file. This has been raised on hygeo as a potential fix.

https://github.com/dblodgett-usgs/hygeo/issues/15

dblodgett-usgs commented 4 years ago

Note that there is a "realized_catchment" attribute on the waterbody_data.geojson features that should provide this information.

dblodgett-usgs commented 4 years ago

@hellkite500 We need to step back from this a bit. I'm realizing I missed a really important nuance regarding waterbodies and flowpaths that I need to correct. I summarized the issue here. https://github.com/dblodgett-usgs/hygeo/issues/21

Working to clean this up now. At the end of the day, I think this means that we will no longer mix waterbody ids into catchment stuff at all. For the case where we have hydrologic routing, we shouldn't be thinking of the conveyance as a waterbody at all -- it is the flowpath realization of the catchment.

I think this may mean that the flowpaths may actually get the catchment id and as we go forward and need distinct waterbody features we will add a new identifier scheme that will relate to catchments at hydrologic locations along flowpaths.

dblodgett-usgs commented 4 years ago

I heavily rewrote the README here: https://github.com/dblodgett-usgs/hygeo and added this uml:

Additionally, I pretty heavily refactored the naming conventions in hygeo to focus on flowpath rather than waterbody. We haven't tackled waterbodies yet -- having only done flowpaths.

The instance you point out above, @hellkite500 -- will now be cat-88 when we get things updated over here.

Let's talk more about what testing data we need to update and what we should do otherwise to make sure these changes get updated in this repo.