Implement GLOFAS dataset - #498
Conversation
|
Hey there, I'm still new to the github workflow, so please let me know how I can improve :) |
79878e8 to
33ef0bf
Compare
ekatef
left a comment
ekatef
left a comment
There was a problem hiding this comment.
Great @StuberSimon! Looks a very good progress, and happy to see GloFAS is being implemented. Have taken a liberty to do a preliminary code review and added a few comments hoping to assist you in getting used to github. My general impression that you are getting things perfectly right and the implementation looks quite neat in general.
Thank you @StuberSimon, looks a great contribution! From my users' perspective, I strongly confirm that having GloFAS would be an amazing feature 🙂 Have added a few preliminary technical comments while leaving more in-depth analysis for @euronion who has much deeper understanding of For the next step, do you need any support? |
|
Hi @StuberSimon, @ekatef, and @euronion First off, great work on this PR. I completely agree with @ekatef that having GloFAS natively integrated is a fantastic and much-needed feature for the community. I'm jumping into this conversation since @Asdominet34 and I have worked on improving the hydromodeling in PyPSA-Eur, by firstly connecting it with GloFAS, "calibrating" it, and then validating it with the real hydro production in Europe. Before pushing the work, we would like to finalize the pumped-storage logic and the working paper we had in mind. I think that there is space for collaboration. What do you think about organising a call next week? |
|
Hi @ValeMTo that sounds great! |
Hi @simon, thanks for your message! Unfortunately next week is quite busy on our side as well, so we won’t be available then. We’ll get back to you shortly with our availability starting from the following week. Sorry again and looking forward to connecting! |
coroa
left a comment
coroa
left a comment
There was a problem hiding this comment.
Sorry for dragging my feet on this PR. Actually, i retract my earlier statement that this has no space within atlite. I think you showed that this can be accommodated here in a way that is useful and backwards compatible.
A couple of changes would be good:
- abstract common code shared between era5.py and glofas.py in some cds_helper.py module
- clean up the coordinates and variable names in use (ie. comments by @ekatef )
- maybe an evaluation of potential heuristics for selecting the correct discharge grid cells
| for plant in plants.itertuples(): | ||
| # Extract the discharge time series for the nearest point | ||
| inflow.loc[dict(plant=plant.Index)] = discharge.sel( | ||
| x=plant.lon, y=plant.lat, method="nearest" | ||
| ) |
There was a problem hiding this comment.
My previous look into these datasets suggested that it is quite easy to miss the correct river cells due to small misalignments in the datasets, so i'd expect one would like to have some more sophisticated find closest river cells which are actually part of the river, rather than find closest cell heuristic. And that this would need testing.
There was a problem hiding this comment.
After some quick thinking probably the best thing would be a snapping to the largest value in the static uparea map (gridded data with the total catchment area of each cell).
That is available from here: https://confluence.ecmwf.int/display/CEMS/Auxiliary+Data
This could also be a pre-processing step on the powerplant dataset for which we provide a helper function that has to be called before.
The only problem is then that when your plant is right before a fork you would latch onto the larger river.
There was a problem hiding this comment.
Hey @coroa thanks for the tip!
I found solving this (especially the fork/larger river problem) quite difficult.
I think I found a working, though not perfect and quite complicated solution.
As the full solution requires some major code additions, I'm not sure if it fits in atlite or this pr.
You can find a first draft and results of these changes here: StuberSimon#1
Note that those changes will still need some preprocessing done in pypsa-earth:
- Calculating expected upstream area of plants (needs head from GHR)
- Getting river names for plants and mapping them (from GHR to OSM)
- Passing those to atlite snapping function, which will combine area matching and river name snapping to find the right glofas cell for each plant
-> This shouldn't be too hard, as GHR and OSM are already used in pypsa-earth. Only the name mapping needs to be done by the user.
I would suggest one of the following options:
- Keep snapping to nearest cell, leave it for future improvements to be solved
- Option 1, implement solution for this in pypsa-earth
- Add only largest upstream area snapping functionality, leave fork/larger river problem to future improvements
- Implement whole package (upstream area and river name matching) in this pr
- Option 1, but I'll implement the whole package in a follow up pr
I would personally prefer option 5. It would keep this pr small and provide a more accurate river cell snapping. I am planning substantial hydro functionality improvements in pypsa-earth after this, and fixing the snapping problem before would be important for those upgrades too.
I would love to hear what you and others (@ekatef) think about that!
|
Hello! @StuberSimon if you'd need any support with additional testing, happy to assist. We are currently working with GloFAS for one of the projects, and there is a bit of struggle exactly with finding the river route as @coroa has mentioned. |
# Conflicts: # .gitignore # atlite/convert.py # atlite/datasets/era5.py
FabianHofmann
left a comment
FabianHofmann
left a comment
There was a problem hiding this comment.
to give this a little push, I am merging this latest master into here. I will follow up with more comments later
- fix add_finalizer(ds, target) call in glofas retrieve_data (was missing ds) - make zip extraction tmpdir-safe via Path(target).parent - align cds_helper helpers with master's typed, lint-clean versions - add type annotations, Returns/Raises docstrings, and logging/pathlib fixes
- restore hydrobasins as 3rd positional arg, make module/time keyword-only so existing positional callers (cutout.hydro(plants, basins)) keep working; collapse the duplicated auto/glofas/era5 dispatch into one path - point era5 request-tracking url back to the CDS server (was pointing at the EWDS/GloFAS server) - drop code-restating comments in glofas retrieve_data
|
Hey @coroa @ekatef @FabianHofmann thank you so much for reviewing! |
|
Hey @coroa are you all right with merging this pr and addressing the river snapping problem in a follow-up pr? |
|
Hello @StuberSimon, looks a great work you have done. On my side, I can confirm you have answered all the technical comments I have shared. For the snapping problem, I have an impression we are struggling with an intrinsic complexity of river topology. Thank you for the investigation @StuberSimon, the pictures are quite insightful to illustrate that the algorithm can interfere with the real topology in a quite sophisticated way. My expectation is that actual effects will strongly depend on a region and it's orography. Checking state-of-the-art, I see that a recent work [Li et all 2025] is using five common types of river topology and it looks like different patterns can lead to different side effects of the algorithm. So, completely agree that it requires some consideration on how to deal with this complexity. Personally, I very much like the idea by @coroa on having a helper function to be applied on the powerplants dataframe. Given results of testing you have done @StuberSimon, I expect that some additional work may be needed to understand what is the best way to deal with that. There is also a point of expandable hydro which adds even more complexity but that is another story. For now, agree with @StuberSimon that it could be a good to agree on the next steps. That is summer and not sure what are availabilities for discussions on this PR. In the meantime, I wonder if we can launch an experimental hydro-focused repo to investigate how the algorithm works for different regions. I can coordinate on creating a dedicated PyPSA-Earth-Hydro repo which we can use with atlite version build from this PR. @StuberSimon could it be an option? |
|
Hi all, jumping into this thread because it overlaps a lot with work my colleagues and I have been doing on the hydro side, which we hadn't yet shared with the PyPSA community. We've built a reproducible pipeline coupling GloFAS/EFAS hydrological reanalyses with PyPSA-Eur to represent run-of-river and reservoir hydropower with plant-specific, physically bounded inflows instead of country-scaled runoff. A central piece is exactly the snapping problem being discussed here: we reconstruct the drainage network from the reanalysis (upstream-area field) and match each plant to a river cell by minimising a combined score that balances geographical proximity and hydraulic scale (rated turbine discharge vs. cell long-term mean flow). It was designed specifically to avoid the main-stem misassignments that pure nearest-cell snapping produces in tributary-rich networks — which seems to be the intrinsic topology issue being pointed at. We also detect reservoir-regulated cells and step upstream to recover the natural inflow. The paper is under review, but there's a preprint here: https://doi.org/10.2139/ssrn.7096701. It might be a useful reference for the experimental hydro-repo idea, and I'd be glad to help if a contribution/PR would be of interest. One caveat on timing: I'm on holiday at the moment, so I'll try to follow up but can't promise detailed responses before september, when I'm back full-time. |
Hello @Asdominet34, and thank you for sharing your insights and the preprint! Looks a great work, and the inflow snapping method you have developed is indeed directly relevant for the discussion in the PR. My feeling is that the method is even more advanced as compared to approaches we have discussed before. So, it makes perfect sense to stick with something simple for an initial implementation. While advancing the inflow snapping could be the next step which will likely require some iterations. |
6 participants
Refs #450
Changes proposed in this Pull Request
Checklist
doc.environment.yaml,environment_docs.yamlandsetup.py(if applicable).doc/release_notes.rstof the upcoming release is included.