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 latitude and longitude to UKV #239

Closed
wants to merge 4 commits into from
Closed

Add latitude and longitude to UKV #239

wants to merge 4 commits into from

Conversation

peterdudfield
Copy link
Contributor

@peterdudfield peterdudfield commented Feb 13, 2025

Pull Request

Description

Add lat and long to UKV.
This is tricky, and I wasn't sure the best way to do this.

A different way to do it, is we explict write to two new data variables, but I dint really want to do this, seemed like more work. I also admit, having the 'a' write mode is a bit dangerous

Should merge into #226, but could go straight to main

How Has This Been Tested?

  • CI tests
  • tested it locally (not the docker image)

Checklist:

  • My code follows OCF's coding style guidelines
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked my code and corrected any misspellings

@peterdudfield peterdudfield requested a review from devsjc February 13, 2025 11:15
@devsjc
Copy link
Collaborator

devsjc commented Feb 13, 2025

Does the downstream forecasting app actually use the 2D latitude and longitude anywhere?

@peterdudfield
Copy link
Contributor Author

Does the downstream forecasting app actually use the 2D latitude and longitude anywhere?

Yea the app uses the latitude and longitude, to reproject onto the same grid that it was trained on.
https://github.com/openclimatefix/uk-pvnet-app/blob/main/pvnet_app/data/nwp.py#L52

@peterdudfield
Copy link
Contributor Author

Does the downstream forecasting app actually use the 2D latitude and longitude anywhere?

Yea the app uses the latitude and longitude, to reproject onto the same grid that it was trained on. https://github.com/openclimatefix/uk-pvnet-app/blob/main/pvnet_app/data/nwp.py#L52

Perhaps I could check that grid first

@peterdudfield
Copy link
Contributor Author

This is in pvnet_app

Screenshot 2025-02-13 at 11 21 10

And this is in the nwp-consumer

Screenshot 2025-02-13 at 11 22 02

@peterdudfield
Copy link
Contributor Author

I think becasue x and y are just integers, I think its important to save the lat and lon here

@devsjc
Copy link
Collaborator

devsjc commented Feb 13, 2025

Thanks for looking. Does it do this for every input or just UKV?

@peterdudfield
Copy link
Contributor Author

Thanks for looking. Does it do this for every input or just UKV?

just UKV

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.

2 participants