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

Interpolation of point forcing data (station-based) using wradlib #315

Merged
merged 66 commits into from
Apr 2, 2025

Conversation

shartgring
Copy link
Collaborator

@shartgring shartgring commented Jan 6, 2025

Issue addressed

Fixes #312

Explanation

Consists of a part in wflow.py for reading and checking the forcing data and station data, and a new workflow in workflow/forcing.py where interpolation takes place.

Checklist

  • Updated tests or added new tests
  • Branch is up to date with main
  • Tests & pre-commit hooks pass
  • Updated documentation if needed

Additional Notes

Interpolation types not working in tests

  • natural neighbor gives error messages and does not work with N = 3 (which was my initial test, it is now N = 8). Not sure why though
  • Cressman interpolation returns a uniform field, so that one needs some extra checking, maybe by directly calling the MetPy function with some test data and seeing what the output looks like. I have only checked the test data which was also processed by hydromt.workflows.precip (the forcing map itself), so maybe the raw data coming out of workflows.forcing.spatial_interpolation needs checking.

01-04-2025: replaced metpy with wradlib

@shartgring
Copy link
Collaborator Author

shartgring commented Jan 6, 2025

The images from the visual inspection of the test data. The title per subplot is the mean value times 1000, and the figure title is the mean over the subplots.

hydromt_wflow_interp_barnes
hydromt_wflow_interp_linear
hydromt_wflow_interp_nearest
hydromt_wflow_interp_rbf

@shartgring shartgring changed the title Interpolation of point forcing data (station-based_ using MetPy Interpolation of point forcing data (station-based) using MetPy Jan 6, 2025
@shartgring
Copy link
Collaborator Author

Allright! After some work the code looks good. Only thing to improve is how the gauge stations are loaded. That code is now copied from setup_gauges, but it can be cleaner and the implementation of the GeoDataset can be improved (linking it to precip_fn and not the stations).

The idea is to leave this PR as a draft as MetPy functionality is not working for natural neighbor and Cressman. It would make sense to switch to https://docs.wradlib.org/en/latest/. However, this will not work with the xarray requirement, and to get rid off the pin we need to upgrade to HydroMT version 1.0. Therefore, the PR will be a draft until we know more about the update to version 1.0.

@shartgring shartgring changed the title Interpolation of point forcing data (station-based) using MetPy Interpolation of point forcing data (station-based) using wradlib Apr 1, 2025
@shartgring
Copy link
Collaborator Author

Replaced metpy with wradlib now that newer xarray versions are supported :)

Tests are passing and all interpolation methods work fine so looks like it is ready for review

@shartgring shartgring marked this pull request as ready for review April 1, 2025 15:48
@shartgring
Copy link
Collaborator Author

I have included an option to filter stations using the basin geometry and a buffer. This might make it easier to work with large datasets. Would it maybe be logical to include somewhere which stations have been used for interpolation? Ar a possible geom / geojson for visualization? I am doubting as they are not part of the model. Listing all the stations in the log is an alternative, but might end up with a long list

Great work! I think it would be very nice to have the meteo stations added as a geojson as well! They are not directly part of the model, but can play quite a big role indirectly, so I can imagine that quickly plotting these locations is very valuable.

I have included it 👍

Copy link
Contributor

@hboisgon hboisgon left a comment

Choose a reason for hiding this comment

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

Very nice improvements @shartgring ! This will definitely become a useful function I think.
I have a few comments mostly on documentation.
And to keep in with the docs, you should not forget to add this function to the hydromt_wflow docs itself :) There are two places for that: api.rst (both for the wflow function and the new forcing workflow) and then update the table for all setup methods for wflow in user_guide/wflow_model_setup.rst

interp_type: str = "nearest",
precip_stations_fn: Optional[Union[str, gpd.GeoDataFrame]] = None,
index_col: Optional[str] = None,
buffer: int = 100,
Copy link
Contributor

Choose a reason for hiding this comment

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

I was maybe not clear but I did check in hydromt and the buffer argument for get_geodataset and get_geodataframe is in meters. So we should update the default value and the docs here.

@shartgring shartgring requested a review from hboisgon April 2, 2025 07:40
Copy link
Contributor

@hboisgon hboisgon left a comment

Choose a reason for hiding this comment

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

Great thanks a lot for the updates! It's good to go now :)

@hboisgon hboisgon merged commit 82a66fe into main Apr 2, 2025
3 of 4 checks passed
@hboisgon hboisgon deleted the point-forcing branch April 2, 2025 10:15
@deltamarnix deltamarnix added this to the Release 0.8 milestone Apr 8, 2025
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.

Use time series to create forcing files
4 participants