Improvement: Mesh handle tutorials - #2348
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2348 +/- ##
=======================================
Coverage 82.36% 82.36%
=======================================
Files 125 125
Lines 20708 20708
=======================================
Hits 17056 17056
Misses 3652 3652 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
lenaploetzke
left a comment
There was a problem hiding this comment.
Partial review. Thank you for your work, this is really nice and my comments are mainly documentation! :)
Co-authored-by: lenaploetzke <70579874+lenaploetzke@users.noreply.github.com>
lenaploetzke
left a comment
There was a problem hiding this comment.
Review adapt callback file
|
Could you please have a look at the failing workflows? |
lenaploetzke
left a comment
There was a problem hiding this comment.
Most things are just typos and capitalization.
Co-authored-by: lenaploetzke <70579874+lenaploetzke@users.noreply.github.com>
| auto mesh = build_mesh<mesh_type> (comm, uniform_level); | ||
| /* Write the mesh to a vtu file. */ | ||
| t8_global_productionf (" [t8 step 3 Mesh handle] \n"); | ||
| t8_global_productionf (" [t8 step 3 Mesh handle] Writing adapted mesh to vtu file: step3_adapted_mesh.vtu\n"); |
There was a problem hiding this comment.
Would be better to introduce a variable for the name and reusing it! That way if you change the name, you do not have to change it everywhere :)
| t8_global_productionf (" [mesh_step3] Writing adapted mesh to vtu file: step3_adapted_mesh.vtu\n"); | ||
| t8_global_productionf (" [mesh_step3] \n"); | ||
| t8_mesh_handle::write_mesh_to_vtk (*mesh, "step3_adapted_mesh.vtu"); |
There was a problem hiding this comment.
Please introduce a variable file_name or something like this for the vtu file name
| #include "t8_mesh_tutorials_common.hxx" /** Adaption function definition used for this tutorial. */ | ||
| #include <memory> | ||
|
|
||
| /** Build our adapted mesh by transferring the adaption parameters and adapting once with our adapt_callback_sphere function. |
There was a problem hiding this comment.
| /** Build our adapted mesh by transferring the adaption parameters and adapting once with our adapt_callback_sphere function. | |
| /** Build our adapted mesh by transferring the adaption parameters and adapting once with the adapt_callback_sphere function defined in \ref t8_mesh_tutorials_common.hxx. |
| * according to our own criterion. | ||
| * | ||
| * The geometry (coarse mesh) is again a cube, this time modelled with | ||
| * 6 tetrahedra, 6 prisms and 4 cubes. |
There was a problem hiding this comment.
| * 6 tetrahedra, 6 prisms and 4 cubes. | |
| * 6 tetrahedra, 6 prisms and 4 hexahedra. |
I know cube is also used in the forest tutorial but we do not use "cube" for elements in general.
| std::unique_ptr<TMeshClass> | ||
| build_mesh (sc_MPI_Comm comm, int level) | ||
| { | ||
| /* Generate a hybrid hypercube, made out of cubes, prisms etc. */ |
There was a problem hiding this comment.
| /* Generate a hybrid hypercube, made out of cubes, prisms etc. */ | |
| /* Generate a hybrid hypercube, made out of hexahedra, prisms etc. */ |
| build_mesh (sc_MPI_Comm comm, int level) | ||
| { | ||
| /* Generate a hybrid hypercube, made out of cubes, prisms etc. */ | ||
| auto mesh = t8_mesh_handle::handle_hypercube_hybrid_uniform_default<TMeshClass> (level, comm); |
There was a problem hiding this comment.
Do you think it is clearer if we also build the mesh in the main just as in the forest step 3 tutorial? And have the function only for adaption? Maybe it is clearer to isolate only the part that is new for this tutorial.
| * \param mesh The initial mesh to adapt. | ||
| */ | ||
| void | ||
| create_partitioned_balanced_mesh (const std::unique_ptr<mesh_type>& mesh) |
There was a problem hiding this comment.
Same as for adapt: we do not create a mesh here, we partition & balance an existing mesh
| * \param mesh The initial mesh to adapt. | ||
| */ | ||
| void | ||
| create_ghost_mesh (const std::unique_ptr<mesh_type>& mesh) |
| void | ||
| create_ghost_mesh (const std::unique_ptr<mesh_type>& mesh) | ||
| { | ||
| /* Creating the ghost layers. */ |
There was a problem hiding this comment.
| /* Creating the ghost layers. */ | |
| /* Set flag such that ghost layer is created on commit. */ |
| true, false, false); | ||
|
|
||
| /** | ||
| * GHOST MESH |
There was a problem hiding this comment.
| * GHOST MESH | |
| * GHOST LAYER |
| #include <mesh_handle/mesh_io.hxx> /** Used to export mesh to vtk files. */ | ||
| #include <mesh_handle/concepts.hxx> /** Include this to use c++ concepts related to the mesh handle. This can be used to constraint the template parameters to only allow mesh handle classes. */ | ||
| #include <t8_types/t8_vec.hxx> /** t8 vector dataclass. */ | ||
| #include "t8_mesh_tutorials_common.hxx" /** Default adaption function. */ |
There was a problem hiding this comment.
| #include "t8_mesh_tutorials_common.hxx" /** Default adaption function. */ | |
| #include "t8_mesh_tutorials_common.hxx" /** Adaption function definition used for this tutorial. */ |
Describe your changes here:
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).