Conversation
…mplex id out of the meshes
…mplex id out of the meshes
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #844 +/- ##
=======================================
Coverage 77.17% 77.17%
=======================================
Files 476 479 +3
Lines 23658 23698 +40
Branches 2642 2641 -1
=======================================
+ Hits 18259 18290 +31
- Misses 5388 5397 +9
Partials 11 11
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
daniel-zint
left a comment
daniel-zint
left a comment
There was a problem hiding this comment.
A few comment on the global_id default value and the asserts are a bit inconsistent. Otherwise, lgtm.
|
|
||
| namespace wmtk::autogen::edge_mesh { | ||
|
|
||
| inline Tuple get_tuple_from_simplex_local_vertex_id(int8_t local_id, int64_t global_id = 0) |
There was a problem hiding this comment.
Is it reasonable to have the global_id default to 0? I feel like having no default value makes more sense.
| /* | ||
| namespace { | ||
| template <size_t Dim> | ||
| inline Tuple get_tuple_from_simplex_local_id_T(int8_t local_id, int64_t global_id = 0) |
There was a problem hiding this comment.
Is that commented out code still relevant? If not, it should be deleted.
|
|
||
| inline Tuple get_tuple_from_simplex_local_vertex_id(int8_t local_id, int64_t global_id = 0) | ||
| { | ||
| const auto& arr = autogen::tet_mesh::auto_3d_table_complete_vertex[local_id]; |
There was a problem hiding this comment.
An assert for local_id would be nice. You have it for the edge mesh.
| assert(lvid == local_id); | ||
|
|
||
|
|
||
| if (lvid < 0 || leid < 0 || lfid < 0) { |
There was a problem hiding this comment.
Is that even possible? That sounds like something that should be checked for in a test and not in here.
2 participants
tuple_from_id is mostly handled by autogen but lacked testing - extracted the functions and added some unit tests