NHD network class #584
NHD network class #584
Conversation
…rcings is called. This is needed to update dictionary items in run_sets (i.e., t0)
…o NHDNetwork object for use with loop iterations
|
I will change the name of file NHDNetwork_edited.py once this PR has been approved |
|
|
||
| return qlat_df | ||
|
|
||
| class NHDNetwork(AbstractNetwork): |
There was a problem hiding this comment.
It would be helpful to review this in the context of the diff to the existing NHDNetwork.py (so don't change the file name...). If you perfer doing this for your own development to have the original to look at, I suggest copying and renaming the original and just not adding that file to the git index.
One thing I'm not real sure of here is the potential initialization of the abstract classes members. I'll have to dig in a little deeper and make sure that all members are initialized (they may be, it is just hard to tell from a first cut look)
There was a problem hiding this comment.
Ok thanks. I moved all the changes to NHDNetwork.py and deleted the "edited" file.
|
@kumdonoaa I've made the changes we talked about |
| "_link_lake_crosswalk", "_link_gage_df", "_usgs_lake_gage_crosswalk", | ||
| "_usace_lake_gage_crosswalk", "_diffusive_network_data", | ||
| "_topobathy", "_refactored_diffusive_domain", "_refactored_reaches", | ||
| "_nonrefactored_topobathy", "_segment_index", "_coastal_boundary_depth_df"] |
There was a problem hiding this comment.
I think most variables from lines 78~81 in slots are already defined in slot of AbstractNetwork.py. In addition, these variables are specified as either property or abstractmethod in the file. I think we don't specify the attributes in a subclass that are already specified in the slots of the base class. That is, by instantiating the base class, more compact data structure from slot is automatically inherited.(??)
| 'musx' : 'MusX', | ||
| 'cs' : 'ChSlp', | ||
| } | ||
| ) | ||
| terminal_code = supernetwork_parameters.get("terminal_code", 0) | ||
| mask = supernetwork_parameters.get("mask_file_path", None) | ||
| mask_layer = supernetwork_parameters.get("mask_layer_string", None) |
There was a problem hiding this comment.
Lines 99~ 122 are already defined in def build_connections of nhd_network_utilities_v02.py, which is called in
t-route/src/troute-nwm/src/nwm_routing/preprocess.py
Lines 211 to 213 in f6293e2
So, we don't need to include these lines here.
| compute_parameters, | ||
| data_assimilation_parameters | ||
| ) | ||
|
|
||
| break_points = {"break_network_at_waterbodies": break_network_at_waterbodies, | ||
| "break_network_at_gages": break_network_at_gages} |
There was a problem hiding this comment.
break_points is only used at line 178 that has been commented out. I am not sure if we need to keep this variable.
| @@ -97,138 +127,132 @@ def __init__(self, supernetwork_parameters, waterbody_parameters=None, restart_p | |||
| break_network_at_gages = supernetwork_parameters.get( | |||
| "break_network_at_gages", False | |||
| ) | |||
There was a problem hiding this comment.
In t-route of the main branch, break_network_at_gages is defined and used at def nwm_network_preprocess of preprocess.py. So, we may remove the lines 127 ~ 129 as the function is called in lines 135~160.
| compute_parameters, | ||
| data_assimilation_parameters | ||
| ) | ||
|
|
There was a problem hiding this comment.
good idea to make it simpler.
| hybrid_parameters, | ||
| self._segment_index, | ||
| cpu_pool, | ||
| self._t0, |
There was a problem hiding this comment.
good idea to make the forcing data preparation further modular. However, coastal_boundary_depth_df is currently prepared as time-series so it can be called only once, not every time loop, which is redundant. We can utilize this code when hourly schism data is completely ready (currently, the hourly data has incorrect date or time information).
* initial commit. edits to NHDNetwork and tools for preprocessing network construction * added feature that makes changes to run_sets when network.assemble_forcings is called. This is needed to update dictionary items in run_sets (i.e., t0) * added new_nhd_q0() and update_waterbody_water_elevation() functions to NHDNetwork object for use with loop iterations * added new_t0() function to NHDNetwork for loop iterations * updated NHDNetwork.py with all changes for use with current troute methods * cleaned up slots and properties * fixed indentations, added self to __init__ * only create coastal_boundary_depth_df once * removed @properties that are already in AbstractNetwork.py Co-authored-by: Sean Horvath <[email protected]> Co-authored-by: Sean Horvath <[email protected]> Co-authored-by: Sean Horvath <[email protected]> Co-authored-by: Sean Horvath <[email protected]>
This builds off of and substitutes PR#580. I created a new PR to separate construction of NHDNetwork (this PR) and refactoring of main.py (PR not issued yet) which will be able to use this new network object and the new data assimilation object (from PR#579).
Edits to NHDNetwork class to fit with current implementation of t-route master branch. Network is created based on an abstract class (though I'm not sure if I'm really utilizing the abstract class design...). This network objects creates and contains all network relevant information, as well as q_lateral forcing data and coastal_boundary_depth data for hybrid simulations. Function to create forcing data is internal to this network object so it can be called repeatedly during loop iterations.
This works with the current version of t-route that reads all data from files. Will need to be refactored to work with ngen model engine.
Additions
Removals
Changes
Testing
Screenshots
Notes
Todos
Checklist
Testing checklist
Target Environment support
Accessibility
Other