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

Added alpha-beta projection to cubed sphere. #87

Merged

Conversation

odlomax
Copy link

@odlomax odlomax commented Jan 25, 2022

This PR predominantly adds an (x, y) to (alpha, beta) projection for tile t. Here, (alpha, beta) is defined strictly as the angular coordinates described by Ronchi et al., (1996).

In practice, the transform is a simple shift and rotate for xy coordinates internal to tile t. A non-linear stretch is applied for external points (i.e. halo xy coordinates).

The following example considers tile t=0 of the LFRic cubed sphere dual mesh, with halo = 3. The first image shows the xy projection. The second image gives the (alpha, beta) projection.

dual_halo_before
dual_halo_after

This required changes to several classes, summarised as follows:

  • grid::Tiles added tileCenter and tileJacobian which return the xy centre and d_xy/d_alphabeta Jacobian for each tile.
  • meshgenerator::detail::cubedsphere::Jacobian2 removed class and added equivalent to util namespace.
  • meshgenerator::detail::cubedsphere::NeighbourJacobian updated this class to use the new Tiles methods. This minor refactoring therefore serves as a test for the changes.
  • CubedSphereGrid added method to return a reference to the stored CubedSphereProjectionBase
  • redistribution::detail::RedistributionGeneric removed rank <= 3 constraint. Not related to projection, but it was an easy fix.

This transform is currently only implemented for the equiangular cubedsphere projection. It is also likely that I've introduced some unnecessary redundancy to the CubedSphereProjectionBase and Tiles classes. However, given that we intend to refactor the cubed sphere grid at some point, we'll eventually need to refactor these classes too.

@wdeconinck
Given that this branch and feature/cubed_sphere_halo_modification both relate to halos, it probably makes sense to merge the two for the upstream PR.

closes #81

@codecov
Copy link

codecov bot commented Jan 25, 2022

Codecov Report

Merging #87 (384d4a0) into feature/cubed-sphere-base2 (6289bf7) will increase coverage by 0.40%.
The diff coverage is 93.21%.

Impacted file tree graph

@@                      Coverage Diff                       @@
##           feature/cubed-sphere-base2      #87      +/-   ##
==============================================================
+ Coverage                       77.59%   78.00%   +0.40%     
==============================================================
  Files                             756      758       +2     
  Lines                           51297    52733    +1436     
==============================================================
+ Hits                            39804    41133    +1329     
- Misses                          11493    11600     +107     
Impacted Files Coverage Δ
src/atlas/grid/Tiles.h 100.00% <ø> (ø)
src/atlas/grid/detail/tiles/FV3Tiles.cc 93.63% <0.00%> (-1.74%) ⬇️
src/atlas/grid/detail/tiles/FV3Tiles.h 100.00% <ø> (ø)
src/atlas/grid/detail/tiles/LFRicTiles.h 100.00% <ø> (ø)
src/atlas/grid/detail/tiles/Tiles.cc 62.50% <ø> (ø)
src/atlas/grid/detail/tiles/Tiles.h 100.00% <ø> (ø)
...shgenerator/detail/CubedSphereDualMeshGenerator.cc 95.23% <ø> (ø)
...hgenerator/detail/cubedsphere/CubedSphereUtility.h 100.00% <ø> (ø)
.../projection/detail/CubedSphereEquiAnglProjection.h 71.42% <ø> (ø)
...projection/detail/CubedSphereEquiDistProjection.cc 65.51% <0.00%> (-10.49%) ⬇️
... and 44 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6289bf7...384d4a0. Read the comment docs.

@odlomax odlomax linked an issue Jan 26, 2022 that may be closed by this pull request
@odlomax odlomax requested a review from mo-lormi January 26, 2022 15:08
Copy link

@MarekWlasak MarekWlasak left a comment

Choose a reason for hiding this comment

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

Can you rename Matrix.h as Matrix2DAndJacobians.h?
Also why is the xy 2alphabeta projections and inverse using different if test conditions? Is it due to non-uniqueness close to the corners?

@odlomax
Copy link
Author

odlomax commented Feb 1, 2022

Can you rename Matrix.h as Matrix2DAndJacobians.h? Also why is the xy 2alphabeta projections and inverse using different if test conditions? Is it due to non-uniqueness close to the corners?

Matrix2DAndJacobians.h is a bit of a mouthful, but I agree it should be called something other than just Matrix.h. Perhaps SquareMatrix.h.

The alphabeta2xy transform doesn't have the invalid corner regions that the xy2alphabeta transform does. But it is degenerate when abs(alpha) == abs(beta) > 45, so a bit of care it needed there.

@odlomax odlomax merged commit 91ff680 into feature/cubed-sphere-base2 Feb 2, 2022
wdeconinck pushed a commit that referenced this pull request Feb 4, 2022
wdeconinck added a commit that referenced this pull request Feb 4, 2022
* develop: (21 commits)
  Add new yml file for variable resolution projection To run the update_uid.py under linux I should add the option shell=True in Popen
  Make VortexRollup function publicly available, and remove it from various places (#87)
  More testing and implementations for VariableResolutionProjection (#89)
  Fixup PointXYZ operator /= to be bitreproducible with PointXYZ::div
  Add *= and /= operators to PointXYZ with scalar
  ATLAS-345 Use new eckit::linalg::LinearAlgebraSparse and eckit::linalg::LinearAlgebraDense API (see ECKIT-585)
  Prevent optimisation with NVHPC compiler in VariableResolutionProjection
  Adapt atlas_test_field_missingvalue as there is no standard on comparisons with NaN
  Workaround for Cray 8.7 compiler bug resulting in FE_DIVBYZERO in atlas_test_jacobian in Release build
  ATLAS-327 Workaround for Cray 8.7 compiler bug with CXX flags "-O2 -hfp1 -g -DNDEBUG"
  Fixup previous commit
  Allow use of build_cells_remote_index() when halo was already built
  Apply clang-format
  Addressed reviewer comments.
  Permitted variable sized halos on functionspaces.
  Added ghost field to CubedSphereCellColumns functionspace.
  ATLAS-344 Fix atlas_test_healpixmeshgen CI with nvhpc-21.9
  ATLAS-344 Fix MatchingMeshPartitionerLonLatPolygon to work with non-matching domains
  ATLAS-344 PolygonXY with Point2 member variables
  ATLAS-344 Printing of util::PolygonCoordinates
  ...
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.

Adjust cubed-sphere dual mesh halo positions
2 participants