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 Jacobian and wind interpolation #99

Closed

Conversation

odlomax
Copy link
Contributor

@odlomax odlomax commented Mar 24, 2022

This PR implements the Projection::jacobian method for the equiangular cubed-sphere. There are three versions of this function:

  • projection::Jacobian Proection::jacobian(const PointXY& xy) This returns the Jacobian ∂(x, y) / ∂(lon, lat).
  • projection::JacobianProection::jacobian(const PointXY& xy, idx_t t) This returns the Jacobian ∂(x, y) / ∂(lon, lat) for tile t. This is needed to perform transforms on halo points.
  • projection::Jacobian Proection::alphabetaJacobian(const Point2& alphabeta, idx_t t) This returns the Jacobian ∂(alpha, beta) / ∂(lon, lat) for tile t. As above, but rotated into the (alpha, beta) coordinate system.

Two tests have been added:

  1. Convergence test, added to tests/projection/test_cubedsphere_projection.cc
    This makes sure that xy + J(x, y) * h(lon, lat) converges quadratically as the step size h(lon, lat) tends to zero.
  2. A demonstration of how the Jacobian can be used to improve the accuracy of (u, v) wind interpolation, especially at the poles
    (added to tests/interpolation/test_interpolation_cubedsphere.cc). Here, the following operations are performed:
    • An analytic (u, v) field is generated for a C48 cubed sphere grid.
    • The (u, v) is transformed to (v_alpha, v_beta) via the alphabetaJacobian method.
    • (v_alpha, v_beta) is interpolated to an O48 Gaussian grid field.
    • The new (v_alpha, v_beta) is transformed back to (u, v) on the new grid.
    • The adjoint of the above is also tested.

The figures below show the output u and v field from the wind interpolation test.
u
v

The two figures below show the absolute interpolation error, i.e. the vector distance between (u_interp, v_interp) and (u_analytic, v_analytic). The colour scale is logarithmic.

  • Field error_field_0 shows the error when we naively interpolate u and v as if they were scalar fields. Note the excessive error
    at the poles (visible near the top of the sphere) where there is a discontinuity in the direction of the (lon, lat) unit vectors.
    error_0
  • Field error_field_1 shows the error when we perform the above wind transform. Note the absence of the polar error in the previous field.
    error_1

@wdeconinck
Copy link
Member

Please integrate the develop branch as PR #98 is merged in now.

@odlomax odlomax marked this pull request as ready for review March 28, 2022 13:39
@odlomax
Copy link
Contributor Author

odlomax commented Mar 28, 2022

Please integrate the develop branch as PR #98 is merged in now.

Done!

odlomax and others added 4 commits April 4, 2022 16:35
* develop:
  Quad2D: Added fused multiply-add to discriminant calculation (ecmwf#102)
  FieldSet::has() to replace FieldSet::has_field()
  Config::json() function
  Create Array using ArraySpec only (containing datatype)
  Add Datatype to ArraySpec
  Cosmetic changes to UnstructuredBilinearLonLat
  Change Quad2D's PointXY arguments to Point2
  Make GridToolsArray backend work with mixed indexing types
@codecov-commenter
Copy link

codecov-commenter commented Apr 7, 2022

Codecov Report

Merging #99 (a0c0c3e) into develop (a3980f7) will increase coverage by 0.08%.
The diff coverage is 95.13%.

@@             Coverage Diff             @@
##           develop      #99      +/-   ##
===========================================
+ Coverage    77.93%   78.02%   +0.08%     
===========================================
  Files          781      781              
  Lines        53503    53726     +223     
===========================================
+ Hits         41696    41918     +222     
- Misses       11807    11808       +1     
Impacted Files Coverage Δ
src/atlas/grid/Tiles.h 100.00% <ø> (ø)
src/atlas/grid/detail/tiles/Tiles.cc 50.00% <0.00%> (-12.50%) ⬇️
src/atlas/grid/detail/tiles/Tiles.h 100.00% <ø> (ø)
.../projection/detail/CubedSphereEquiAnglProjection.h 71.42% <ø> (ø)
...projection/detail/CubedSphereEquiDistProjection.cc 57.57% <0.00%> (-7.95%) ⬇️
.../projection/detail/CubedSphereEquiDistProjection.h 57.14% <ø> (ø)
...tlas/projection/detail/CubedSphereProjectionBase.h 100.00% <ø> (ø)
src/atlas/grid/Tiles.cc 78.37% <50.00%> (-3.44%) ⬇️
...polation/method/cubedsphere/CubedSphereBilinear.cc 70.73% <100.00%> (+0.73%) ⬆️
...rpolation/method/cubedsphere/CubedSphereBilinear.h 87.50% <100.00%> (+1.78%) ⬆️
... and 6 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 a3980f7...a0c0c3e. Read the comment docs.

@wdeconinck
Copy link
Member

@odlomax Again a very good contribution and high quality code!
I'm very pleasantly surprised how the StructuredMeshGenerator just worked with the odd partition shapes. No doubt however some failing corner cases would still pop up when stress-testing.
Are you happy for me to do a squash merge?

@odlomax
Copy link
Contributor Author

odlomax commented Apr 8, 2022

@wdeconinck Thank you for your kind words, you've been a great help! Squash and merge away! 😀

@@ -82,7 +82,7 @@ class Jacobian : public std::array<std::array<double, 2>, 2> {
return Jacobian{(*this)[1][1], -(*this)[0][1], -(*this)[1][0], (*this)[0][0]} * (1. / determinant());
}

Jacobian transpose() const { return Jacobian{(*this)[1][1], (*this)[0][1], (*this)[1][0], (*this)[0][0]}; }
Jacobian transpose() const { return Jacobian{(*this)[0][0], (*this)[1][0], (*this)[0][1], (*this)[1][1]}; }
Copy link
Member

Choose a reason for hiding this comment

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

😱

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that was a bit of a head-scratcher! 😂

// wind transform. Then the transform is applied to the entire field,
// *including* the halo.

sourceFunctionspace.parallel_for(util::Config("include_halo", true), [&](idx_t idx, idx_t t, idx_t i, idx_t j) {
Copy link
Member

Choose a reason for hiding this comment

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

very nice!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

According @MarekWlasak I'm obsessed with std::tie. I think he's right! 😂

@wdeconinck
Copy link
Member

Squash-merged in develop with commit 1b08c23

@wdeconinck wdeconinck closed this Apr 8, 2022
@wdeconinck wdeconinck added the Status: Merged PR has been merged in alternative way label Apr 8, 2022
@odlomax odlomax deleted the feature/cubed_sphere_jacobian branch April 8, 2022 12:32
wdeconinck added a commit that referenced this pull request Apr 21, 2022
* release/0.29.0: (49 commits)
  Version 0.29.0
  GitHub Actions CI: enable OpenMP for C/C++
  ATLAS-355 Workaround macOS OpenMP problems with 'omp task'
  ATLAS-354 bamboo using proj/8.2.1
  ATLAS-354 Introduce FindPROJ.cmake for proj installations based on autotools
  ATLAS-354 Compatibility with proj >= 8
  Add missing include <string>
  Restore compatibility with eckit < 1.18.5
  Support cubed-sphere wind interpolation (#99)
  Fix Jacobian::transpose() introduced in PR #93
  GitHub actions CI: reduce available MPI_SLOTS for macOS builds to 4
  Quad2D: Added fused multiply-add to discriminant calculation (#102)
  FieldSet::has() to replace FieldSet::has_field()
  Config::json() function
  Create Array using ArraySpec only (containing datatype)
  Add Datatype to ArraySpec
  Cosmetic changes to UnstructuredBilinearLonLat
  Change Quad2D's PointXY arguments to Point2
  Make GridToolsArray backend work with mixed indexing types
  Apply clang-format
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Merged PR has been merged in alternative way
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants