respec / HSPsquared

Hydrologic Simulation Program Python (HSPsquared)
GNU Affero General Public License v3.0
43 stars 17 forks source link

_iwater_() fails when IWAT-STATE1 variables are missing #96

Closed rburghol closed 1 year ago

rburghol commented 1 year ago

I get the following error when running a simulation with an impervious land use that lacks the IWAT-STATE1 block. This may be a bug in the version of the model that I am using (Ches. Bay model 5.3.2, hspf 11.0), or, it mayu be something that is generally allowable in HSPF, so I thought I'd post this up. These are initial condition vars.

I amended code to get around this, by checking if the vars exist in the ui list, and if not, to initialize them to zero. But if this is NOT a bug, obviously I want to fix our UCI generation routine to be standard compliant.

git diff: Changes to HSP2/IWATER.py

     # initial conditions
-    rets = ui['RETS']
-    surs = ui['SURS']
+    rets = 0
+    if 'RETS' in ui:
+        rets = int(ui['RETS'])
+    surs = 0
+    if 'SURS' in ui:
+        surs = int(ui['SURS'])

Anyone have insight here?

Traceback (most recent call last):
  File "/home/rob/.local/bin/hsp2", line 11, in <module>
    load_entry_point('HSPsquared', 'console_scripts', 'hsp2')()
  File "/opt/model/HSPsquared/HSP2tools/HSP2_CLI.py", line 60, in main
    mando.main()
  File "/home/rob/.local/lib/python3.8/site-packages/mando/core.py", line 208, in __call__
    return self.execute(sys.argv[1:])
  File "/home/rob/.local/lib/python3.8/site-packages/mando/core.py", line 204, in execute
    return command(*a)
  File "/opt/model/HSPsquared/HSP2tools/HSP2_CLI.py", line 26, in run
    hsp2main(io_manager, saveall=saveall, jupyterlab=jupyterlab)
  File "/opt/model/HSPsquared/HSP2/main.py", line 209, in main
    errors, errmessages = function(io_manager, siminfo, ui, ts)
  File "/opt/model/HSPsquared/HSP2/IWATER.py", line 69, in iwater
    errors = _iwater_(ui, ts)                       # run IWATER simulation code
  File "/home/rob/.local/lib/python3.8/site-packages/numba/typed/dictobject.py", line 738, in impl
    raise KeyError()
PaulDudaRESPEC commented 1 year ago

The HSP2 code is designed to populate the initial state variables with the defaults if those initial state variables are not explicitly set -- this happens down within readUCI. Are you using readUCI to populate the h5 file? If so, you may indeed have discovered a bug.

Also worth noting, the default values for the IWAT-STATE1 variables are not exactly zero, from the HSPF manual:

--------------------------------------------------------------------------------
Symbol         Fortran name   Format  Def     Min     Max     Units   Unit syst
--------------------------------------------------------------------------------
<iwat-state1>  RETS           2F10.0  .001    .001    100     inches  Engl
                                      .025    .025    2500    mm      Metric
               SURS                   .001    .001    100     inches  Engl
                                      .025    .025    2500    mm      Metric
rburghol commented 1 year ago

Thanks @PaulDudaRESPEC -- I am using hsp2 import_uci to populate the h5 file, and I will look into readUCI().
FWIW I am using the most recent pull from the "develop" branch.

PaulDudaRESPEC commented 1 year ago

Right, import_uci calls readUCI. I'll run a test on that when I get a chance and see if I can reproduce the problem you report.

rburghol commented 1 year ago

@PaulDudaRESPEC I looked into the readUCI function and found that I could eliminate the error by checking for the presence of RETS, and if absent, set it and SURS to a default value of 0.001 per the entry you showed from the HSPF manual. Code 1: git diff: Changes to HSP2tools/readUCI.py

--- a/HSP2tools/readUCI.py
+++ b/HSP2tools/readUCI.py
@@ -207,6 +207,10 @@ def readUCI(uciname, hdfname):
                 df['PETMIN'] = 0.35
                 df['PETMAX'] = 40.0
                 df.to_hdf(store, path, data_columns=True)
+            if 'RETS' not in df.columns:   # didn't read IWAT-STATE1 table
+                df['RETS'] = 0.001
+                df['SURS'] = 0.001
+                df.to_hdf(store, path, data_columns=True)
PaulDudaRESPEC commented 1 year ago

@rburghol Indeed the fix you propose will take care of the specific problem you identified, so that's great.

There's a more general issue where the code designed to populate default values (around line 328) is not working properly in this case and probably others -- I'm going to dig into that a little further to better understand why sometimes that code doesn't work as intended.

PaulDudaRESPEC commented 1 year ago

@rburghol I just pushed a code fix to the develop branch to address this issue in readUCI, in a more generic way for any missing 'initial state variable' table. Maybe you'd like to check this out when you get a chance.

rburghol commented 1 year ago

Thanks @PaulDudaRESPEC - I'll pull that asap and report back.

rburghol commented 1 year ago

Paul - I am sorry to delay response, this fix resolved our issue entirely. I had been so busy using it that I forgot to check back.