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.

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

Add function to modelbuilder to generate observations along coastline #578

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

Tammo-Zijlker-Deltares
Copy link
Collaborator

Fixes #575

This function is now added, but needs TLC and cleaning up. To do for Tammo

Fixes #575

This function is now added, but needs TLC and cleaning up. To do for Tammo
Copy link
Collaborator

@veenstrajelmer veenstrajelmer left a comment

Choose a reason for hiding this comment

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

Thanks for sharing this feature! I have placed some review comments for a first iteration. Let me know if you do not have time to do this, since I might pick it up in that case. I do prefer the reviewer approach though. Besides the comments, could you also check the sonarcloud bugs? There are some unused variables.

bbox = [lon_min, lat_min, lon_max, lat_max],
crs = "EPSG:4326")
linestrings = [polygon.boundary for polygon in coastline['geometry']]
linstrings_cropped = [shapely.ops.clip_by_rect(linestring, lon_min, lat_min, lon_max, lat_max) for linestring in linestrings]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Possible to do this on coastlines_gdb already? Would prevent many lines of code

#Build GeoSeries with points along coastline
cpoints = gpd.GeoSeries()
for index, line in shp_clipped.iterrows():
if 'MULTILINESTRING' in str(line.values):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Concat all in multilinesrring or split all in linestring, so we prevent this if-else loop. I think you can use the private function _pointlike_to_geodataframe_points() (from dfm_tools.interpolate_grid2bnd import _pointlike_to_geodataframe_points at top of modelbuilder.py). This also checks for the amount of polylines in the file and whether all names are unique.


#%% calculate face center x, y and z
print('interpolating cell centers from nodes')
ds_net = xr.open_dataset(file_nc)
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you provide the netfile, I suggest to also use uds.grid.bounds instead of providing lat/lon min/max manually

net_face_x = []
net_face_y = []
net_face_z = []
for face in ds_net.mesh2d_nFaces:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Gebruik hier dfmt.uda_nodes_to_faces(), in #644 toegevoegd aan dfm_tools

bool_valid_cells = (net_faces['z']<-threshold_mindepth)

#creating kdtree with valid cell centers (cartesian coordinates)
def xlonylat2xyzcartesian(data):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Might be related to dfmt.calc_dist_haversine(). Let's discuss this later, but at least place in separate function already.

net_face_z.append(net_face_z_sel)

#Make dictionary with cell centers
net_faces = pd.DataFrame({'x':net_face_x, 'y':net_face_y, 'z':net_face_z})
Copy link
Collaborator

Choose a reason for hiding this comment

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

Eerst zvar ophalen, filter op diepte (sel) en alleen die xy coords bewaren

obs_snapped['y'] = data_celcenxy_validsel['y'].values
obs_snapped = obs_snapped.drop_duplicates()

#Create obs file for dflowfm
Copy link
Collaborator

Choose a reason for hiding this comment

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

Buiten functie halen

tree = KDTree(data_celcenxy_valid_cart[['x_cart','y_cart','z_cart']])

def dist_to_arclength(chord_length):
"""
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as for xlonylat2xyzcartesian

@sonarcloud
Copy link

sonarcloud bot commented Oct 25, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 6 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add function to modelbuilder to generate observations along coastline
2 participants