-
Notifications
You must be signed in to change notification settings - Fork 51
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.
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
updating nexus on parquet file #817
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to be careful with averaging the water depth. Look at flowpath ID 2427759. This flowpath lies just downstream of a waterbody. Waterbodies tend to always have much greater depths than flowpaths, so when you average depths of this ID and the waterbody above it, the depth is artificially inflated. Depth before averaging, ((287, d) 1.462409):
But after doing the conversion from flowpaths to nexus points (22.146202m):
|
||
def crosswalk_nex_flowpath_poi(self): | ||
self._nexus_dict = dict() | ||
for key, values in self._connections.items(): | ||
for value in values: | ||
new_key = f"nex-{value}" | ||
new_value = f"wb-{key}" | ||
if new_key not in self.nexus_dict: | ||
self._nexus_dict[new_key] = [] | ||
self._nexus_dict[new_key].append(new_value) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NHD network doesn't have nexus points, so I'm not sure this is needed here. There should be some method for NHD just to prevent errors, but NHD output will always be indexed by flowpath IDs because that's all it has.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's True, and already the output of NHD is by flowpath IDs, I just created this artificially dictionary to use the same function (updated_flowveldepth
) I created for analyzing cross-walk between nexus point and flowpath ids in hyfeature. (To avoid repetition of a same function)
daf4867
to
e7bc8af
Compare
@jameshalgren Would you look at this PR, I think this PR and your PR (#842) are related |
if nexus_dict: | ||
location_ids = prefix_ids + '-' + df['location_id'].astype(str) | ||
else: | ||
location_ids = df['location_id'].astype(str) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a breaking change. For example if your configuration looked like:
output_parameters:
parquet_output:
parquet_output_folder: output/
prefix_ids: wb
The location_id
column would no longer be prefixed with wb-
(e.g. wb-111
would now be 111
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
parquet_output_folder: output/ | ||
configuration: short_range | ||
prefix_ids: nex | ||
# parquet_output: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you elaborate on why this was commented out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to simplify the yaml file. So if user need that, can uncomment it
This PR updates the parquet file output if its nexus point is requested, It creates a crosswalk between upstream and downstream (as nex-, wb-) and if there are more than one segment going to a nexus points it add up the flowrate and average the waterdepth.
Additions
Removals
Changes
Testing
Screenshots
Notes
Todos
Checklist
Testing checklist
Target Environment support
Accessibility
Other