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

fix: swap meshgrid dimension ordering in xarray grid creation #249

Merged
merged 2 commits into from
Mar 27, 2025

Conversation

timothyas
Copy link
Contributor

Description

A solution (that feels somewhat hacky) to #245

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update

Issue Number

Closes #245

Code Compatibility

  • I have performed a self-review of my code

Code Performance and Testing

  • I have added tests that prove my fix is effective or that my feature works
  • I ran the complete Pytest test suite locally, and they pass

Dependencies

  • I have ensured that the code is still pip-installable after the changes and runs
  • I have tested that new dependencies themselves are pip-installable.

Documentation

N/A

  • My code follows the style guidelines of this project
  • I have updated the documentation and docstrings to reflect the changes
  • I have added comments to my code, particularly in hard-to-understand areas

Additional Notes

When running the test case provided in #245, I get what I'd expect: that the latitudes and longitudes are different due to the different ordering of the arrays. Here's what I see now in the output:

Screenshot 2025-03-24 at 10 27 43 AM

And I'm printing the first 10 values of longitudes and latitudes to show that the difference is not just because of something wacky, but because of the dimension ordering.

@timothyas timothyas changed the title hacky solution for meshgrid issue in grid.py fix: swap meshgrid dimension ordering in xarray grid creation Mar 24, 2025
b8raoult
b8raoult previously approved these changes Mar 24, 2025
@b8raoult
Copy link
Collaborator

Can you re-fork datasets and republish the PR? I cannot merge.

@b8raoult
Copy link
Collaborator

Do you have an example of each type of mesh that I could add to the tests?

@timothyas
Copy link
Contributor Author

timothyas commented Mar 25, 2025

@b8raoult I just rebased the branch for the PR and resolved conflicts, so it should be able to be merged. Also I generalized it to work with the case where lat/lon need to be renamed.

As for test datasets, if you want something that has the exact same data, then the scripts and datasets I provided in #245 could work. Otherwise, here are two out in the wild:

1. Here's a recipe to pull in an ERA5 dataset with dimensions ("longitude", "latitude")

dates:
  start: 2000-01-01T00:00:00
  end: 2000-01-02T18:00:00
  frequency: 6h

input:
  join:
      - xarray-zarr:
          url: "gs://weatherbench2/datasets/era5/1959-2023_01_10-6h-64x32_equiangular_conservative.zarr"
          param:
            - 2m_temperature

      - xarray-zarr:
          url: "gs://weatherbench2/datasets/era5/1959-2023_01_10-6h-64x32_equiangular_conservative.zarr"
          param:
            - temperature
          level:
            - 100
            - 500
            - 1000

      - forcings:
          template: ${input.join.0.xarray-zarr}
          param:
            - cos_latitude
            - cos_longitude

2. And here's a recipe to pull in Replay data, which has dimensions ("latitude", "longitude")

dates:
  start: 2020-01-01T00
  end: 2020-01-01T12
  frequency: 3h

input:
  join:
  - xarray-zarr:
        url: "gs://noaa-ufs-gefsv13replay/ufs-hr1/0.25-degree-subsampled/03h-freq/zarr/fv3.zarr"
        param: [tmp2m]
        flavour:
            rules:
                latitude:
                    name: grid_yt
                longitude:
                    name: grid_xt
                time:
                    name: time
  - xarray-zarr:
        url: "gs://noaa-ufs-gefsv13replay/ufs-hr1/0.25-degree-subsampled/03h-freq/zarr/fv3.zarr"
        param: [tmp]
        pfull: [97.8232650756836, 505.6520690917969]
        flavour:
            rules:
                latitude:
                    name: grid_yt
                longitude:
                    name: grid_xt
                time:
                    name: time
                level:
                    name: pfull
            levtype: pl

I just tested these by taking this PR and rebasing it onto 4abd0cc, but this PR as it is will continue to fail given the issue noted in #248 (although I'm still unsure of what the original cause of that is).

@b8raoult b8raoult merged commit 938f3c9 into ecmwf:main Mar 27, 2025
15 of 17 checks passed
@timothyas timothyas deleted the bugfix/meshgrid-latlon-hack branch March 28, 2025 01:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Potential issue with xarray grid creation
3 participants