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

Cubed sphere 2 grid builder #253

Merged

Conversation

mo-jonasganderton
Copy link
Contributor

@mo-jonasganderton mo-jonasganderton commented Jan 13, 2025

Description

Part 2 of implementing a new Cubed Sphere grid (follows #245).
These changes so far include:

  • CS2 grid builder
  • CS2 grid factory
  • CS2 projection

Impact

Once everything else is also implemented, this makes the cubed sphere more maintainable.

Checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have run the unit tests before creating the PR

@wdeconinck
Copy link
Member

Please rebase on develop and mark as ready for review when you want me to have a look.

@mo-jonasganderton mo-jonasganderton force-pushed the feature/cubed_sphere_2_grid_builder branch 5 times, most recently from c5c4b7a to 5211606 Compare February 6, 2025 17:14
@mo-jonasganderton mo-jonasganderton marked this pull request as ready for review February 6, 2025 17:14
@mo-jonasganderton mo-jonasganderton force-pushed the feature/cubed_sphere_2_grid_builder branch from 5211606 to 984524d Compare February 7, 2025 10:13
@wdeconinck
Copy link
Member

Hi @mo-jonasganderton, with the new CS approach the Projection can be arbitrary and probably should not derive from a dedicated one for the CS.
You can add a test where the projection passed is rotation, and verify that the {lonlat} projected coordinates are no longer matching the original {xy} coordinates as they are rotated.

Copy link

Caution

This pull request contains changes to GitHub workflows!
Proceed with caution and if not sure, contact your GitHub admin.

1 similar comment
Copy link

Caution

This pull request contains changes to GitHub workflows!
Proceed with caution and if not sure, contact your GitHub admin.

@mo-jonasganderton mo-jonasganderton force-pushed the feature/cubed_sphere_2_grid_builder branch from f6e44d8 to 3c2da9c Compare February 12, 2025 17:00
Copy link

Caution

This pull request contains changes to GitHub workflows!
Proceed with caution and if not sure, contact your GitHub admin.

@mo-jonasganderton
Copy link
Contributor Author

Hi @wdeconinck, indeed Olly mentioned removing the specific projection too, so now I have done so. Extra tests for rotation added too.

Copy link

Caution

This pull request contains changes to GitHub workflows!
Proceed with caution and if not sure, contact your GitHub admin.

Copy link
Member

@wdeconinck wdeconinck left a comment

Choose a reason for hiding this comment

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

Looks good to merge! Thanks @mo-jonasganderton 🙏

@wdeconinck wdeconinck merged commit 0ee57e5 into ecmwf:develop Feb 18, 2025
174 of 175 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants