NOAA-OWP / ngen

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

`wb` in catchment names breaks ngen CFE execution #529

Open jameshalgren opened 1 year ago

jameshalgren commented 1 year ago

The "pre-release" hydrofabric uses two-character wb abbreviations in the naming convention, which breaks code in the ngen preprocessing tools hard-coded to look for three-character cat abbreviations.

e.g., https://github.com/NOAA-OWP/ngen/blob/0e33df6ec43eca05299d84cada707fbe6c9dfb1a/utilities/data_conversion/csv2catchmentnetcdf.py#L67

jameshalgren commented 1 year ago

ping @igarousi

mattw-nws commented 1 year ago

@jameshalgren There are some places we're going to have to work on this and related issues for the pre-release HF (particularly I think the new inx- and cnx- names for nexuses) and I found your issue when starting to look into those...but the example you cited is specific to making netcdf forcing files from CSVs... those should still be cat-X, and the divides table still has IDs in that format... were you hitting this problem in a different context?

jameshalgren commented 1 year ago

@mattw-nws -- I'll check our scripting more carefully. I see the cat designation in the divides table, but the wb designation in the network/topological tables. It might be that we are combining those and producing an ID from the topology table that we could just code back to the expected three letter abbreviation, Will double check with @igarousi, who first ran into the issue and realized what was going on.

mattw-nws commented 1 year ago

Yeah I'm not sure that quite tracks with what I'm looking at for issues though I may be missing something.

So, in the oldest hydrofabrics, cat-XX would have toid (really to_id) of nex-YY, and nex-YY would point to a toid of cat-YY, which would in turn point to nex-ZZ ... so cat->nex->cat->nex. In most versions for the past year+, cat-XX points to nex-YY, but nex-YY points to wb-ZZ... and then wb-ZZ points to nex-AA--that is, nothing links back to catchments... so it's

cat        cat        cat
 v          v          v
nex > wb > nex > wb > nex > wb ...

which probably makes more sense and I think follows the HY_Features standard where the previous topology did not.

However, crucially, we have a test case in data/gauge_01073000 that includes this kind of linkage and ngen itself--at least--works*. I think the partitionGenerator may not work properly--needs retesting--but the realization in there works properly and it includes CFE.... so yeah I think I need more info on the precise symptoms.

jameshalgren commented 1 year ago

@mattw-nws: Very helpful explanation. Diving in and will update soon. @hellkite500, we might discuss next time we get a chance.