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

Improve robustness of conservative spherical polygon interpolation for high resolution meshes #130

Conversation

sbrdar
Copy link
Collaborator

@sbrdar sbrdar commented May 2, 2023

Fixing #129

  • improvement in polygon intersection (more robust Sutherland-Hodgman algorithm)
  • print out the worst under-covering and over-covering of target polygons
  • output in JSON format
    Remaining issue from Development of efficient conservative spherical polygon interpolation #129
  • searching of candidate source polygons to intersect a give target polygon should not be done via std::set and comparison of centroids to determine if two polygons are same. The coefficient ATLAS_COMPAREPOINTXYZ_EPS_FACTOR should be set to 1e4 for high grid res, but 1e8 for low mesh. This has to be removed in the future version.

@FussyDuck
Copy link

FussyDuck commented May 2, 2023

CLA assistant check
All committers have signed the CLA.

@sbrdar sbrdar requested a review from wdeconinck May 2, 2023 12:30
@github-actions
Copy link

github-actions bot commented May 2, 2023

Private downstream CI failed.
Workflow name: private downstream ci
View the logs at https://github.com/ecmwf-actions/private-downstream-ci/actions/runs/4861239577.

@andreapiacentini
Copy link

andreapiacentini commented May 2, 2023

Hi, sorry I did not enter the details of these changes (and honesty I probably wouldn't understand them), but I take the opportunity to share with you what we are dealing with in OASIS and that has already been fixed in ESMF and YAC interpolators (not sure about XIOS). Some finite volumes models (e.g the FESOM2 ocean model https://fesom.de/models/fesom20/) define the control volumes with an enhanced dual mesh also passing by the midpoints of the primal mesh edges (cf the C_i cell in Fig 2. from https://fesom2.readthedocs.io/en/latest/geometry.html).
This leads to non convex cells and ESMF and YAC adapted their Sutherland-Hodgman algorithm so that it can deal with concave cells (at least for one of the two grids).
If you aim at having a generic spherical conservative interpolation, that could be an interesting enhancement for Atlas too.
FESOM2 grids can be found here https://gitlab.awi.de/fesom
If you just need the dual grid for the CORE2 resolution (the one really needed for flux interpolations) I can provide you with the one is causing problems in the "pre-Sutherland-Hodgman" SCRIP library.

@codecov-commenter
Copy link

codecov-commenter commented May 2, 2023

Codecov Report

Patch coverage: 88.85% and project coverage change: +0.66 🎉

Comparison is base (7d4fac6) 77.36% compared to head (dd1503c) 78.02%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #130      +/-   ##
===========================================
+ Coverage    77.36%   78.02%   +0.66%     
===========================================
  Files          810      810              
  Lines        58708    58896     +188     
===========================================
+ Hits         45420    45956     +536     
+ Misses       13288    12940     -348     
Impacted Files Coverage Δ
...ctured/ConservativeSphericalPolygonInterpolation.h 100.00% <ø> (ø)
...tured/ConservativeSphericalPolygonInterpolation.cc 87.47% <79.76%> (+27.08%) ⬆️
src/atlas/util/ConvexSphericalPolygon.cc 92.14% <89.79%> (+2.88%) ⬆️
src/tests/util/test_convexsphericalpolygon.cc 99.48% <99.31%> (-0.52%) ⬇️
src/atlas/util/ConvexSphericalPolygon.h 100.00% <100.00%> (ø)
...s/interpolation/test_interpolation_conservative.cc 100.00% <100.00%> (ø)

... and 4 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@sbrdar
Copy link
Collaborator Author

sbrdar commented May 2, 2023

Hi Andrea. Thanks for your good remarks! We do aim for a generic conservative interpolation, MPI-parallel & 1st/2nd order. Data on mesh can be either in the cell centres or on the vertices of a mesh. We intersect only convex polygons and polygon edges being great arcs, so we have to split non-convex polygon around a vertex into convex quads. This has worked well for us so far. I would be grateful if you can provide us with the one example causing problems in the "pre-Sutherland-Hodgman" SCRIP library. Thanks again!

@andreapiacentini
Copy link

andreapiacentini commented May 2, 2023

if you can provide us with the one example

Done!
At least if you mail has the usual format [email protected]
Let me know if you get the message and the file transfer. The grids coordinates are in a scrip like grids file. There were loader around at the time when I tested Atlas interpolations, but at the time we did not talk much of non-convex dual finite volumes grids!
Don't hesitate to get in touch for any questions or brainstorming. I can also put you in touch with the FESOM developers: they certainly know much more than I do about these grids. Sometimes my Italian flavor of written English collides with their German flavor, but it would be easier for you !

@wdeconinck wdeconinck force-pushed the feature/highres-conservative-spherical-polygon-interpolation branch from a31bd02 to dd1503c Compare May 2, 2023 16:59
@github-actions
Copy link

github-actions bot commented May 2, 2023

Private downstream CI succeeded.
Workflow name: private downstream ci
View the logs at https://github.com/ecmwf-actions/private-downstream-ci/actions/runs/4863898021.

@wdeconinck wdeconinck changed the title Feature/highres conservative spherical polygon interpolation Fix robustness of conservative spherical polygon interpolation for high resolution meshes May 3, 2023
@wdeconinck wdeconinck changed the title Fix robustness of conservative spherical polygon interpolation for high resolution meshes Improve robustness of conservative spherical polygon interpolation for high resolution meshes May 3, 2023
sbrdar and others added 3 commits May 3, 2023 10:06
…CTOR env var

New default 1e8*eps instead of 1e4*eps.
For previous behaviour:
 ATLAS_COMPAREPOINTXYZ_EPS_FACTOR=1e4

For high resolution we want to have it to 1e4, for low resolution 1e8
The detection mechanism of source points needs to be replaced with something deterministic that
does not rely on comparing points.
@wdeconinck wdeconinck force-pushed the feature/highres-conservative-spherical-polygon-interpolation branch from dd1503c to 5233ae5 Compare May 3, 2023 08:07
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.

Great work @sbrdar !!!

@wdeconinck wdeconinck merged commit 8440795 into develop May 3, 2023
@github-actions
Copy link

github-actions bot commented May 3, 2023

Private downstream CI succeeded.
Workflow name: private downstream ci
View the logs at https://github.com/ecmwf-actions/private-downstream-ci/actions/runs/4869722949.

@wdeconinck wdeconinck deleted the feature/highres-conservative-spherical-polygon-interpolation branch November 13, 2023 16:02
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.

5 participants