NOAA-OWP / ngen

Next Generation Water Modeling Engine and Framework Prototype
Other
83 stars 62 forks source link

Error finding forcings for catchment with irregular id format #680

Open robertbartel opened 10 months ago

robertbartel commented 10 months ago

As noted in NOAA-OWP/hydrofabric#28, there's at least one catchment in the current hydrofabric with an id in an unexpected format: cat-7e+05. An error occurs related to this catchment when trying to run the framework, which is how it was discovered:

terminate called after throwing an instance of 'std::runtime_error'
  what():  Forcing data could not be found for 'cat-7e+05'

However, a forcing file does exist at /dmod/datasets/forcing/regridded/regrid_02/csv/cat-7e+05.csv. The forcing portion of the realization config is:

        "forcing": {
           "file_pattern": "{{id}}.csv",
           "path": "/dmod/datasets/forcing/regridded/regrid_02/csv/",
           "provider":  "CsvPerFeature"
        }
robertbartel commented 10 months ago

Similar issue with cat-1e+06 of _nextgen05.gpkg.

robertbartel commented 10 months ago

This may be unrelated to the irregular id, though that isn't fully clear.

I ran a similar configuration on a different machine (same formulation types, same hydrofabric, different file paths) with debugging (I suppose that also means the CMake build type was different). After a great many formulations were constructed, eventually we arrived at one when the ifstream failed and we had that same runtime error. Curiously, this time the catchment was cat-702271. Once again, the CSV file does actually exist and has data.

I tried to call rdstate() on the stream to get more detail on the failure but ran into this. Same thing happened when I try to call good(). goodbit itself was 0 and failbit was 4. I'm guessing it's because I don't have symbols for iostate but I'm not sure, so throwing it in here.

expression failed to parse:
error: Couldn't lookup symbols:
  __ZNKSt3__19basic_iosIcNS_11char_traitsIcEEE7rdstateB6v15006Ev
hellkite500 commented 10 months ago

For the original issue, its most likely related to the regex pattern building here. We subsititue {{id}} in the string with the id string, and in this case the string contains regex specific characters, namely + which is causing the match to fail later. I think I can work a quick patch to escape the character.

PhilMiller commented 10 months ago

Could we go the other direction, of setting a whitelist of catchment/nexus name characters, and actually enforce somewhat reasonable input? [A-Za-z0-9-_] should be more than sufficient.

We probably shouldn't be constructing regex patterns like this, with interpolated strings (security concerns aside ...), but if we are, limiting the interpolated bits keeps everything simpler.