Feature cmesh vertex connectivity
Describe your changes here:
Implemented two classes for cmesh global vertex id handling.
tree to vertex - Each local tree and ghost stores the list of global ids of its vertices. vertex to tree - Each global vertex stores a list of all local trees and ghost trees and vertices that share this global vertex.
We implemented setter and getter functions and tests.
This feature is still in development.
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
-
[ ] The reviewer executed the new code features at least once and checked the results manually
-
[ ] The code follows the t8code coding guidelines
-
[ ] New source/header files are properly added to the Makefiles
-
[ ] The code is well documented
-
[ ] All function declarations, structs/classes and their members have a proper doxygen documentation
-
[ ] All new algorithms and data structures are sufficiently optimal in terms of memory and runtime (If this should be merged, but there is still potential for optimization, create a new issue)
Tests
- [ ] The code is covered in an existing or new test case using Google Test
Github action
-
[ ] The code compiles without warning in debugging and release mode, with and without MPI (this should be executed automatically in a github action)
-
[ ] All tests pass (in various configurations, this should be executed automatically in a github action)
If the Pull request introduces code that is not covered by the github action (for example coupling with a new library):
- [ ] Should this use case be added to the github action?
- [ ] If not, does the specific use case compile and all tests pass (check manually)
Scripts and Wiki
- [ ] If a new directory with source-files is added, it must be covered by the
script/find_all_source_files.scpto check the indentation of these files. - [ ] If this PR introduces a new feature, it must be covered in an example/tutorial and a Wiki article.
Licence
- [x] The author added a BSD statement to
doc/(or already has one)
Note: Inlining get_global_vertices requires us to move the implementation in the header, in turn requiring us to include "t8_cmesh_types" in the header. Since t8_cmesh_types is an internal header that must not be exposed to the user, this means that now "t8_cmesh_vertex_conn_tree_to_vertex.hxx" is then definitely an internal header. This is okay, since the user interface/public header should only be "t8_cmesh_vertex_connectivity.hxx" or even only "t8_cmesh.hxx".
- address code review comments
- clean up
- documentation
- Fixed bugs so that now the tests pass (at least i hope, the tests are not finished running while i write this)
- did not implement the optimization suggestion (i.e. unstructured map, etc) due to time reasons
- I was not sure if we can update this branch with main or not, so i did not do it for now.
If you are happy with the functionality i suggest to merge this now. We can add the suggested optimizations later (but should make issues out of them before they get lost).
removed draft status
https://github.com/DLR-AMR/t8code/pull/960#discussion_r1986352073
I adressed this by writing a doxygen style docstring for the function.
In order to clean up our folder structure i moved all cmesh vertex connectivity related cmesh files into a subfolder "src/t8_cmesh/t8_cmesh_vertex_connectivity/". This declutters the src/t8_cmesh directory.
https://github.com/DLR-AMR/t8code/pull/960#discussion_r1986352021
Resolved this by merging the dev notes with the actual class documentation.
TODO: @holke reconstruct the DISABLED test, what was happening? Why did i add the #0? Can we ENABLE the test now?
TODO: @holke reconstruct the DISABLED test, what was happening? Why did i add the #0? Can we ENABLE the test now?
No, we cannot enable the tests now. The disabled tests are part of a test suite that would check all cmesh test examples, but cannot be used currently because we cannot add attributes when deriving a cmesh.
However, i removed the DISABLED keyword, because the tests were never registered with gtest in the first place, but (correctly) commented out with #if 0. Which is not the usual way to do it - we should always opt for DISABLED - but in this case necessary, since the tests refer to a whole Test suite that is not run and to avoid code copying the preprocessor is the better alternative here.
I am done, should be ready to merge. back to @sandro-elsweijer