Feature: OpenFOAM reader [2/n] - Hex meshes without connectivity#2258
Feature: OpenFOAM reader [2/n] - Hex meshes without connectivity#2258sandro-elsweijer wants to merge 9 commits into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2258 +/- ##
==========================================
- Coverage 81.74% 81.21% -0.53%
==========================================
Files 126 127 +1
Lines 20702 20836 +134
==========================================
Hits 16922 16922
- Misses 3780 3914 +134 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
spenke91
left a comment
There was a problem hiding this comment.
Sorry it took so lonf until I found time for the review...
Great to see you are making progress here! 🥳
I only have a bunch of minor suggestions and questions. For the hex construction, I admit I had a hard time following every single step in detail, but I am happy to have a more detailed look next time :-)
| error = error && !read_faces (case_faces_file); | ||
| error = error && !read_owner (case_owner_file); | ||
| error = error && !read_neighbor (case_neighbor_file); | ||
| bool success = 1; |
There was a problem hiding this comment.
I am getting used to using 0 and 1 for bool, but can't we at least use true for initializing bools? 😆
| bool success = 1; | |
| bool success = true; |
| * \return The eclass of the cell | ||
| */ | ||
| t8_eclass_t | ||
| get_cell_eclass (const size_t cell_id) |
There was a problem hiding this comment.
Sorry to play that card again, but why is this function's implementation in the header, not the source file? 🤓
| /* OpenFOAM only has 3D cells, so we do not check 0-2 dimensional cells. */ | ||
| switch (m_cell_faces[cell_id].size ()) { | ||
| case 4: | ||
| /* This cell is hopefully a tet with 4*3=12 points. */ |
There was a problem hiding this comment.
| /* This cell is hopefully a tet with 4*3=12 points. */ | |
| /* This cell should be a tet with 4*3=12 points. */ |
| num_points += m_face_points[face.first].size (); | ||
| } | ||
|
|
||
| /* OpenFOAM only has 3D cells, so we do not check 0-2 dimensional cells. */ |
There was a problem hiding this comment.
| /* OpenFOAM only has 3D cells, so we do not check 0-2 dimensional cells. */ | |
| // Distinguish shapes based on the number of faces and points. | |
| // Note: OpenFOAM only has 3D cells, so we do not check 0-2 dimensional cells. |
| return T8_ECLASS_PYRAMID; | ||
| break; | ||
| case 6: | ||
| /* This cell is hopefully a hex with 6 * 4 = 24 points. */ |
There was a problem hiding this comment.
| /* This cell is hopefully a hex with 6 * 4 = 24 points. */ | |
| /* This cell should be a hex with 6 * 4 = 24 points. */ |
But if you prefer, you can also stick to hopefully, it is funnier ;-)
| * the next vertex is vertex 1 and therefore vertex 0 of face 1. Otherwise, the previous point is vertex 0 of face 1. | ||
| * We iterate cyclic over the face vertices. So if we arrive at the end, we begin at the start. */ | ||
| auto next_vertex | ||
| = (found_face_0_vertex_0 + 1) == check_face->end () ? check_face->begin () : found_face_0_vertex_0 + 1; |
There was a problem hiding this comment.
| = (found_face_0_vertex_0 + 1) == check_face->end () ? check_face->begin () : found_face_0_vertex_0 + 1; | |
| = ( (found_face_0_vertex_0 + 1) == check_face->end () ? check_face->begin () : found_face_0_vertex_0 + 1 ); |
| * We iterate cyclic over the face vertices. So if we arrive at the end, we begin at the start. */ | ||
| auto next_vertex | ||
| = (found_face_0_vertex_0 + 1) == check_face->end () ? check_face->begin () : found_face_0_vertex_0 + 1; | ||
| /* Check if face 0 contains this vertex */ |
There was a problem hiding this comment.
It looks like it is searching through face 1 though?
| } | ||
| else { | ||
| const auto previous_vertex | ||
| = found_face_0_vertex_0 == check_face->begin () ? check_face->end () : found_face_0_vertex_0 - 1; |
There was a problem hiding this comment.
| = found_face_0_vertex_0 == check_face->begin () ? check_face->end () : found_face_0_vertex_0 - 1; | |
| = (found_face_0_vertex_0 == check_face->begin () ? check_face->end () : found_face_0_vertex_0 - 1); |
| /* ------------------------- 4. Add remaining four faces ------------------------- */ | ||
|
|
||
| /* Lastly, we have to match the rest of the faces so we can add the boundaries to the cmesh and | ||
| * to get the orientations and faces for neighbor linkage. */ |
There was a problem hiding this comment.
| * to get the orientations and faces for neighbor linkage. */ | |
| * to get the orientations and faces for neighbor linkage. */ |
| //t8_face_normal = t8_eclass_face_orientation[eclass][1]; | ||
| //orientation.second = (face_normals[*face_id] == t8_face_normal); |
There was a problem hiding this comment.
| //t8_face_normal = t8_eclass_face_orientation[eclass][1]; | |
| //orientation.second = (face_normals[*face_id] == t8_face_normal); |
Closes #2259
Describe your changes here:
This PR adds the feature to read OpenFOAM hex only meshes without connectivity.
The internal data structures of the reader are used to assemble hex cells and add them to a cmesh.
The provided example can now be used to read a hex mesh like the mesh from the
pitzDailySteadyOpenFOAM example.All these boxes must be checked by the AUTHOR before requesting review:
Documentation:,Bugfix:,Feature:,Improvement:orOther:.All these boxes must be checked by the REVIEWERS before merging the pull request:
As a reviewer please read through all the code lines and make sure that the code is fully understood, bug free, well-documented and well-structured.
General
Tests
If the Pull request introduces code that is not covered by the github action (for example coupling with a new library):
Scripts and Wiki
scripts/internal/find_all_source_files.shto check the indentation of these files.License
doc/(or already has one).