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

Enable MeshBuilder to set up the Grid #152

Merged

Conversation

fmahebert
Copy link
Contributor

Add a config argument to the MeshBuilder's call operator. This config can be used to request setting the mesh's grid, either as an UnstructuredGrid from the available coordinates in the MeshBuilder, or as a grid of choice according to the config.

This resolves #150 for the most part. Some edge cases are discussed there that I did not code up in this PR (could be added later if needed, though).

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! A few minor changes requested.

src/atlas/mesh/MeshBuilder.cc Outdated Show resolved Hide resolved
src/tests/mesh/test_mesh_builder_parallel.cc Outdated Show resolved Hide resolved
src/tests/mesh/test_mesh_builder_parallel.cc Outdated Show resolved Hide resolved
src/atlas/mesh/MeshBuilder.cc Outdated Show resolved Hide resolved
src/atlas/mesh/MeshBuilder.cc Outdated Show resolved Hide resolved
src/atlas/mesh/MeshBuilder.cc Outdated Show resolved Hide resolved
Comment on lines 111 to 113
} else {
throw_Exception("In MeshBuilder: can't validate a grid of type " + grid.type() + " from config");
}
Copy link
Member

Choose a reason for hiding this comment

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

We can in principle use the iterator here with random access as last resort. It will work for any grid, but could be slower than the 2 above methods.

auto point = *(grid.lonlat().begin()+global_indices[i]-1);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good suggestion, I've added this

@codecov-commenter
Copy link

codecov-commenter commented Jul 27, 2023

Codecov Report

Patch coverage: 89.31% and project coverage change: +0.03% 🎉

Comparison is base (9ba6f09) 78.01% compared to head (25f4b09) 78.04%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #152      +/-   ##
===========================================
+ Coverage    78.01%   78.04%   +0.03%     
===========================================
  Files          817      817              
  Lines        52464    52588     +124     
===========================================
+ Hits         40928    41043     +115     
- Misses       11536    11545       +9     
Files Changed Coverage Δ
src/atlas/mesh/Mesh.h 88.88% <ø> (ø)
src/atlas/mesh/MeshBuilder.cc 88.70% <78.46%> (-11.30%) ⬇️
src/tests/mesh/test_mesh_builder.cc 100.00% <100.00%> (ø)
src/tests/mesh/test_mesh_builder_parallel.cc 100.00% <100.00%> (ø)

... and 4 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

fmahebert and others added 2 commits July 27, 2023 13:46
Co-authored-by: Willem Deconinck <wdeconinck@me.com>
Co-authored-by: Willem Deconinck <wdeconinck@me.com>
fmahebert and others added 5 commits July 27, 2023 13:47
Co-authored-by: Willem Deconinck <wdeconinck@me.com>
Co-authored-by: Willem Deconinck <wdeconinck@me.com>
Co-authored-by: Willem Deconinck <wdeconinck@me.com>
Co-authored-by: Willem Deconinck <wdeconinck@me.com>
@fmahebert
Copy link
Contributor Author

@wdeconinck — Thanks for the feedback, and mostly for the extensive planning discussion in the preceding issue!

@wdeconinck wdeconinck merged commit d3d55f4 into ecmwf:develop Jul 31, 2023
79 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.

Set UnstructuredGrid in meshes built by MeshBuilder
3 participants