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

implement a procedure to carry out a regridding from high to low resolution (binning) #191

Merged

Conversation

mo-lormi
Copy link
Contributor

@mo-lormi mo-lormi commented May 3, 2024


Description


This PR is intended to extend the group of the interpolation operations, in particular to:

  • implement a procedure to evaluate the areas of the grid cells (grid type: CS-LFR)
  • implement a procedure to carry out a regridding from high to low resolution (binning)
  • add new tests to the test battery


Rigridding from high to low resolution via binning ...

method: binning
binning matrix: B = N A^T W
area weights matrix: W
interpolation matrix: A
normalization factor matrix: N


Acceptance Criteria (Definition of Done)

All the tests pass.


@mo-lormi
Copy link
Contributor Author

mo-lormi commented May 3, 2024

@wdeconinck , @odlomax This PR covers the implementation of the regridding procedure (from high to low resolution).
At this stage, I implemented a simple test associated with the procedure to generate data in Gmsh format for data visualization. Further tests will be added.

@odlomax
Copy link
Contributor

odlomax commented May 3, 2024

Thanks, @mo-lormi

Could you add some of the outputs to this PR so we can easily have a look?

@mo-lormi
Copy link
Contributor Author

mo-lormi commented May 3, 2024

The following plots refer to two fields:

  1. field: field_01_s ; regridding type: high to low; source grid: CS-LFR-100
  2. field: field_01_t ; regridding type: high to low; target grid: CS-LFR-50

regridding_h2l_field_s
regridding_h2l_field_t

@twsearle
Copy link
Contributor

twsearle commented May 3, 2024

This looks very nice! I am curious as a bystander why you haven't used the interpolation elements. Perhaps if you had it would have worked for any grid? https://github.com/ecmwf/atlas/blob/develop/src/atlas/interpolation/element/Quad2D.h

Maybe out of scope though!

@mo-lormi
Copy link
Contributor Author

mo-lormi commented May 3, 2024


I carried out further testing. The procedure also works for a different type of grid (function space: StructuredColumns).

The following plots refer to two fields:

  1. field: field_02_s ; regridding type: high to low; source grid: O32
  2. field: field_02_t ; regridding type: high to low; target grid: O16

(note -- It seems that a set of contiguous cells is missing in both plots. Thus, further investigation is needed)

regridding_h2l_field02_s
regridding_h2l_field02_t

@wdeconinck
Copy link
Member

@mo-lormi concerning the "missing" part in the Gmsh plot, is nothing to worry about. That's because the StructuredColumns fields are unlike NodeColumns and don't have extra duplicated nodes at 360 degrees.

@mo-lormi
Copy link
Contributor Author

mo-lormi commented May 8, 2024

@mo-lormi concerning the "missing" part in the Gmsh plot, is nothing to worry about. That's because the StructuredColumns fields are unlike NodeColumns and don't have extra duplicated nodes at 360 degrees.

@wdeconinck thanks for this info ... To take into account this point, I will need to update the procedure.

@mo-lormi mo-lormi marked this pull request as ready for review May 10, 2024 08:40
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.

Thank you and sorry for the delayed review. Please see my in-code comments.

src/atlas/interpolation/method/binning/Binning.cc Outdated Show resolved Hide resolved
src/atlas/interpolation/method/binning/Binning.cc Outdated Show resolved Hide resolved
src/atlas/interpolation/method/binning/Binning.cc Outdated Show resolved Hide resolved
inline idx_t N() const { return N_; }

// Return the areas of the faces/cells
Field gridCellArea(const FunctionSpace&) const;
Copy link
Member

@wdeconinck wdeconinck Jun 10, 2024

Choose a reason for hiding this comment

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

In order to encapsulate as much as possible the grid from objects like Field, FunctionSpace, Mesh,
I would avoid this function.

Possibly you could create a different function

double CubedSphere::gridCellArea(idx_t index);

The grid itself should have all possible information needed to compute just a single cell value.

Then where needed fill a container such as std::vector or atlas::Field or ...
But in this case here, I think you could just compute the cellArea within the loop where you compute the weights, avoiding the entire allocation of a container.

Copy link
Contributor Author

@mo-lormi mo-lormi Jun 11, 2024

Choose a reason for hiding this comment

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

@wdeconinck ... good point. However, there is reason why the method CubedSphere::gridCellArea() has been designed and implemented in this way.

So far, at the MO, we have identified at least two workflow steps where this function is needed: one is the evaluation of the area weights matrix within the interpolation/binning procedure (this PR); the other one is the evaluation of the Jc term, a penalty term associated with the gravity wave constraints for the cost function.
Thus, we thought that having a function signature for CubedSphere::gridCellArea() based on the FunctionSpace data type could make it general enough to be used in different contexts.

Copy link
Member

Choose a reason for hiding this comment

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

I managed to boil this down to this standalone function, independent of functionspace

// Area around nodes of cubed sphere mesh
Field CubedSphereNodalArea(const Mesh& mesh) {

  constexpr auto deg2rad = M_PI / 180.;
  const auto& proj = mesh.projection();
  auto lonlat = array::make_view<double, 2>(mesh.nodes().lonlat());
  auto gcell_area_field = Field("grid_cell_areas", make_datatype<double>(), array::make_shape(mesh.nodes().size()));  
  auto gcell_area_fview = array::make_view<double, 1>(gcell_area_field);

  double gcell_area_cs = [&] {
      /******* CUBED_SPHERE SPECIFIC *********/
      // (grid_res * grid_res) = no. of cells on a tile
      ATLAS_ASSERT(CubedSphereGrid(mesh.grid()));
      auto grid_res = CubedSphereGrid(mesh.grid()).N();
      // area of a grid cell (cubed-sphere coord. system)
      return M_PI/(2*grid_res) * M_PI/(2*grid_res);
   }();

  for (size_t i = 0; i < gcell_area_fview.size(); i++) {
    PointLonLat loc = PointLonLat(lonlat(i, atlas::LON), lonlat(i, atlas::LAT));
    double cos_lat = std::cos(deg2rad * loc.lat());
    double grid_jac_det = 1/proj.jacobian(loc).determinant();
    // area of a grid cell (geographic coord. system)
    gcell_area_fview(i) = grid_jac_det * gcell_area_cs * cos_lat ;   /****** USE gcell_area_cs here ******/
  }

  return gcell_area_field;
}

Possibly with more thought this could be made even more independent of CubedSphere.

What happens to the Binning procedure if the combination does not happen to be "grid == CubedSphere && functionspace == NodeColumns" ?

Copy link
Contributor Author

@mo-lormi mo-lormi Jun 11, 2024

Choose a reason for hiding this comment

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

@wdeconinck ... thanks for this.
Where do you think this function should be stored? In which file, class,...?
If you tell me where I should define the function, I will do a test with it. Thanks ...

Copy link
Member

Choose a reason for hiding this comment

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

It could be a mesh-action, similar to https://github.com/ecmwf/atlas/blob/develop/src/atlas/mesh/actions/BuildCellCentres.h
So putting it parallel to this file, and following its design is a good idea.
You can then "register" this field within the mesh.nodes() and could be reused later.

What happens to the Binning procedure if the combination does not happen to be "grid == CubedSphere && functionspace == NodeColumns" ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

functionspace == NodeColumns

The binning procedure works also for "functionspace == StructuredColumns" (grid type: "O").
I implemented a test to check this scenario, see here.

@mo-lormi
Copy link
Contributor Author

mo-lormi commented Jun 13, 2024


@wdeconinck thanks again for the comments and suggestions.
I re-implemented the procedure to evaluate the areas of the grid cells based on your proposal.
Thanks also to @odlomax for helping to identify the cause of a segmentation fault failure.

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.

Thanks so much for this contribution and addressing all my suggestions. 🙏

@mo-lormi
Copy link
Contributor Author

Thanks so much for this contribution and addressing all my suggestions. 🙏

@wdeconinck thanks again.

@wdeconinck wdeconinck merged commit c4d34f8 into ecmwf:develop Jun 17, 2024
21 checks passed
@wdeconinck
Copy link
Member

This will be released in 0.38.0 some time this week.

wdeconinck added a commit that referenced this pull request Jun 20, 2024
* release/0.38.0: (47 commits)
  Update Changelog
  Version 0.38.0
  implement a procedure to carry out a regridding from high to low resolution (binning) (#191)
  Fixes opposite pole coordinates (#202)
  Avoid silent errors accessing Fieldset fields by ambiguous names (#210)
  Made sure cubed-sphere interpolation method always sets metadata. (#208)
  Add fortran interface for node-to-edge connectivity building (#209)
  Removed redundant headers.
  Fixed SphericalVector.
  Fixed StructuredInterpolation2D.
  Added failing tests.
  Update ci.yml to fix notification (#207)
  Add notifications to github workflows
  Remove float in Triag2D intersection algorithm (#203)
  Fixup: use C++17 uncaught_exceptions (extra s in exceptions)
  Disable logging of ATLAS_TRACE during stack unwinding (e.g. when exception is thrown)
  Add GPU offloading capability with OpenACC support to Native WrappedDataStore
  Add C++ OpenACC test for some sanity
  Refactor OpenACC support and implement for Native arrays
  Enable use of CUDA for Native arrays
  ...
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.

4 participants