Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix for diffusive to run with synthetic and topo on NHD #779

Merged
merged 2 commits into from
Jun 14, 2024
Merged

fix for diffusive to run with synthetic and topo on NHD #779

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a266e58
fix for diffusive to run with synthetic and topo on NHD
Jun 7, 2024
f4177d0
move topobathy data processing from MCwithDiffusive to MCwithDiffusiv…
Jun 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions src/troute-network/troute/AbstractNetwork.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -474,20 +474,20 @@ def initialize_routing_scheme(self,):
routing_type = [run_hybrid, use_topobathy, run_refactored]

_routing_scheme_map = {
MCOnly: [False, False, False],
MCwithDiffusive: [True, False, False],
MCwithDiffusiveNatlXSectionNonRefactored: [True, True, False],
MCwithDiffusiveNatlXSectionRefactored: [True, True, True],
MCOnly: [[False, False, False]],
MCwithDiffusive: [[True, False, False],[True, True, False]],
MCwithDiffusiveNatlXSectionNonRefactored: [[True, True, False]],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused here, it looks like you added a [True, True, False] option (line 478), but that already exists as MCwithDiffusiveNatlXSectionNonRefactored (line 479)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is true but MCwithDiffusive was mainly written for using topo data while 'use_topobathy'=False. So, when 'use_topobathy'=False, MCwithDiffusive throws errors in many cases (e.g., L221). So I guess then the fix can be that we keep _routing_scheme_map as before, but keep the changes I made in AbstractNetwork.py. Or, we can move L216~L237 to somewhere under class MCwithDiffusiveNatlXSectionNonRefactored?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, RouteLink file doesn't have column name 'mainstem', we cannot use def _fill_in_missing_topo_data of AbstractRouting.py for NHD. Furthermore, topobathy data isn't available anymore for NHD. So, unless communities try to make their own topodata for NHD, 'MConly' and 'MCwithDiffusive (with use_topobathy=False) are only options to choose for NHD.

MCwithDiffusiveNatlXSectionRefactored: [[True, True, True]],
}

# Default to MCOnly routing
routing_scheme = MCOnly

# Check user input to determine the routing scheme
for key, value in _routing_scheme_map.items():
if value==routing_type:
if routing_type in value:
routing_scheme = key

routing = routing_scheme(self.hybrid_parameters)

(
Expand Down
50 changes: 26 additions & 24 deletions src/troute-network/troute/AbstractRouting.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,32 +213,34 @@ def update_routing_domain(self, dataframe, connections, waterbody_dataframe):
self._diffusive_domain = diffusive_domain_all

# Load topobathy data and remove any links for which topo data cannot be obtained
topobathy_df = self.topobathy_df
missing_topo_ids = list(set(all_links).difference(set(topobathy_df.index)))
topo_df_list = []

for key in missing_topo_ids:
topo_df_list.append(_fill_in_missing_topo_data(key, dataframe, topobathy_df))

if len(topo_df_list)==0:
new_topo_df = pd.DataFrame()
else:
new_topo_df = pd.concat(topo_df_list)
if self.hybrid_params['use_natl_xsections']:
topobathy_df = self.topobathy_df
missing_topo_ids = list(set(all_links).difference(set(topobathy_df.index)))
topo_df_list = []

for key in missing_topo_ids:
topo_df_list.append(_fill_in_missing_topo_data(key, dataframe, topobathy_df))

if len(topo_df_list)==0:
new_topo_df = pd.DataFrame()
else:
new_topo_df = pd.concat(topo_df_list)

bad_links = list(set(missing_topo_ids).difference(set(new_topo_df.index)))
self._topobathy_df = pd.concat([self.topobathy_df,new_topo_df])

# select topo data with minimum value of cs_id for each segment
df = self._topobathy_df
min_cs_id=df.reset_index().groupby('hy_id')['cs_id'].transform('min')
mask = df.reset_index()['cs_id'] == min_cs_id
single_topo_df = df.reset_index()[mask]
single_topo_df.set_index('hy_id', inplace=True)
self._topobathy_df= single_topo_df

for tw in self._diffusive_domain:
#mainstem_segs = self._diffusive_domain[tw]['links']
bad_links = list(set(missing_topo_ids).difference(set(new_topo_df.index)))
self._topobathy_df = pd.concat([self.topobathy_df,new_topo_df])

# select topo data with minimum value of cs_id for each segment
df = self._topobathy_df
min_cs_id=df.reset_index().groupby('hy_id')['cs_id'].transform('min')
mask = df.reset_index()['cs_id'] == min_cs_id
single_topo_df = df.reset_index()[mask]
single_topo_df.set_index('hy_id', inplace=True)
self._topobathy_df= single_topo_df
else:
# Use synthetic channel cross section data instead.
bad_links = []

for tw in self._diffusive_domain:
wbody_ids = waterbody_dataframe.index.tolist()
targets = self._diffusive_domain[tw]['targets'] + bad_links
links = list(reachable(rconn_diff0, sources=[tw], targets=targets).get(tw))
Expand Down
Loading