update LowerColorado with bmi and flowveldepth function implemented #697
Conversation
There was a problem hiding this comment.
Can you rename this file to something like "run_with_BMI.py"? 'all_configs' is misleading as this only runs one configuration
| import sys | ||
| import os | ||
| import glob | ||
| import numpy as np | ||
| import pandas as pd | ||
| import geopandas as gpd | ||
| import pickle | ||
| from datetime import datetime, timedelta | ||
| import time |
There was a problem hiding this comment.
Looks like this file is not using glob, gpd, datetime.datetime, or pickle. Can you remove these imports?
There was a problem hiding this comment.
Maybe undo the changes to this file? Unless we are updating the organization of parameters or the parameters themselves, I think we should leave this as is. Users can adjust the inputs themselves to fit their needs.
There was a problem hiding this comment.
@AminTorabi-NOAA actually, ignore this comment. Changing the output parameters to being on in the yaml file is good I think.
There was a problem hiding this comment.
It looks like all of the output .csv files have the same column names, regardless of the .csv file timestamp. For example, 202304020000.flowveldepth.csv column names are (correctly):
,q_202304020000,v_202304020000,d_202304020000,ndg_202304020000
But 202304020100.flowveldepth.csv column names are (incorrectly):
,q_202304020000,v_202304020000,d_202304020000,ndg_202304020000
Can you make sure these are being updated accordingly with the model time?
There was a problem hiding this comment.
Good catch, Thanks. Done
src/model_DAforcing.py
Outdated
| q0 = values.get('q0') | ||
| waterbody_df = values.get('waterbody_df') | ||
| lastobs_df = values.get('lastobs_df') | ||
| if len(q0) != 0 and len(waterbody_df) != 0 and len(lastobs_df) != 0: |
There was a problem hiding this comment.
Were we going to change this line? q0 should never be empty, right? And I believe _write_lite_restart checks if waterbody_df is empty before trying to write that file. lastobs_df should not impact whether _write_lite_restart is called so should not be here.
There was a problem hiding this comment.
Updated in a way that when write_lite_restart is 1 it go through _write_lite_restart function
src/model_DAforcing.py
Outdated
| if output_parameters.get('stream_output',{}).get('qvd_nudge'): | ||
| #write flowveldepth file | ||
| pass | ||
| # if output_parameters.get('stream_output',{}) is not None and output_parameters.get('stream_output',{}).get('qvd_nudge'): |
There was a problem hiding this comment.
Can you delete this commented out line?
There was a problem hiding this comment.
In addition to the specific comments, I got the following error when I tried to run run_with_BMI.py:
***************test AnA Run***************
Initialize DA Module...
Traceback (most recent call last):
File "/home/sean.horvath/projects/t-route/test/LowerColorado_TX_v4/run_with_BMI.py", line 132, in <module>
run_troute('test AnA')
File "/home/sean.horvath/projects/t-route/test/LowerColorado_TX_v4/run_with_BMI.py", line 38, in run_troute
DAforcing.initialize(bmi_cfg_file = base_path + config_dict[config]['yaml'])
File "/home/sean.horvath/projects/t-route/src/bmi_DAforcing.py", line 117, in initialize
self._model = DAforcing_model(bmi_cfg_file)
File "/home/sean.horvath/projects/t-route/src/model_DAforcing.py", line 84, in __init__
self._usgs_df.
File "/home/sean.horvath/miniconda-template/miniconda3/lib/python3.9/site-packages/pandas/core/frame.py", line 10535, in resample
return super().resample(
File "/home/sean.horvath/miniconda-template/miniconda3/lib/python3.9/site-packages/pandas/core/generic.py", line 8312, in resample
return get_resampler(
File "/home/sean.horvath/miniconda-template/miniconda3/lib/python3.9/site-packages/pandas/core/resample.py", line 1423, in get_resampler
return tg._get_resampler(obj, kind=kind)
File "/home/sean.horvath/miniconda-template/miniconda3/lib/python3.9/site-packages/pandas/core/resample.py", line 1599, in _get_resampler
raise TypeError(
TypeError: Only valid with DatetimeIndex, TimedeltaIndex or PeriodIndex, but got an instance of 'Index'
| q0 = values.get('q0') | ||
| waterbody_df = values.get('waterbody_df') | ||
| lastobs_df = values.get('lastobs_df') |
There was a problem hiding this comment.
Looks like these are no longer used here, right? If that's the case can you delete them?
| self._t0, | ||
| output_parameters['lite_restart'] | ||
| ) | ||
| values['write_lite_restart'] = 0 | ||
| lastobs_output_folder = da_parameters.get('streamflow_da',{}).get('lastobs_output_folder', None) |
There was a problem hiding this comment.
can you add writing lastobs to the if write_lite_restart == 1: block? technically lastobs is a different kind of file, but it's only used to restart our DA module from a previous run, so it essentially acts as a restart file.
|
|
||
| import bmi_troute | ||
| import bmi_DAforcing | ||
| import model_DAforcing |
There was a problem hiding this comment.
Don't need model_DAforcing to be loaded. This is used within bmi_DAforcing. Can you delete this?
A code added in LowerColorado_TX_v4 for running the bmi. It uses the "test_AnA_V4_HYFeature" yaml file.
Additions
Removals
Changes
Testing
Screenshots
Notes
Todos
Checklist
Testing checklist
Target Environment support
Accessibility
Other