Feature vertex ghosts #1353

lukasdreyer · 2025-01-24T12:49:10Z

Describe your changes here:
Adds the needed vertex functionality for vertex ghosts on a single tree to the scheme interface and implements it for the default quad scheme.

Extends the ghost algorithm, so that vertex ghosts on a single tree coarse mesh can be constructed.

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.scp to 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.

License

The author added a BSD statement to doc/ (or already has one)

Tag Label

The author added the patch/minor/major label in accordance to semantic versioning.

Davknapp

In general this looks good. But the behaviour of the tutorial is changed in a way such that it doesn't show the balance algorithm anymore. This should not be the case. I think it is a good idea to make a seperate ghost tutorial step showing the differences between Face and vertex ghost. The current step4 can be a follow up step (step5) to show the balance algo again.

Davknapp · 2025-02-04T11:43:03Z

src/t8_forest/t8_forest.cxx

@@ -2899,7 +2899,7 @@ t8_forest_set_ghost_ext (t8_forest_t forest, int do_ghost, t8_ghost_type_t ghost
 {
  T8_ASSERT (t8_forest_is_initialized (forest));
  /* We currently only support face ghosts */
-  SC_CHECK_ABORT (do_ghost == 0 || ghost_type == T8_GHOST_FACES,
+  SC_CHECK_ABORT (do_ghost == 0 || ghost_type == T8_GHOST_FACES || ghost_type == T8_GHOST_VERTICES,


Please update the error-message, too.

Davknapp · 2025-02-04T11:45:09Z

src/t8_schemes/t8_scheme.hxx

@@ -837,6 +837,33 @@ class t8_scheme {
                       eclass_schemes[tree_class]);
  };

+  inline int


Please document this function.

Davknapp · 2025-02-04T11:47:10Z

src/t8_schemes/t8_scheme.hxx

+                       eclass_schemes[tree_class]);
+  }
+
+  inline void


Please document this function.

Davknapp · 2025-02-04T11:47:23Z

src/t8_schemes/t8_scheme.hxx

+      eclass_schemes[tree_class]);
+  }
+
+  inline void


Please document this function.

Davknapp · 2025-02-04T11:48:51Z

src/t8_schemes/t8_default/t8_default_quad/t8_default_quad.hxx

@@ -546,6 +546,60 @@ class t8_default_scheme_quad: public t8_default_scheme_common<t8_default_scheme_
  void
  element_get_anchor (const t8_element_t *elem, int anchor[3]) const;

+  inline int


Suggested change

inline int

constexpr int

Davknapp · 2025-02-04T11:58:58Z

src/t8_schemes/t8_default/t8_default_quad/t8_default_quad.hxx

+    p4est_quadrant_t *elem = (p4est_quadrant_t *) element;
+    p4est_quadrant_t *desc = (p4est_quadrant_t *) descendant;
+    p4est_qcoord_t coord_offset = P4EST_QUADRANT_LEN (elem->level) - P4EST_QUADRANT_LEN (level);
+    desc->x = elem->x + coord_offset * ((vertex & 1 << 0) >> 0);


Suggested change

desc->x = elem->x + coord_offset * ((vertex & 1 << 0) >> 0);

desc->x = elem->x + coord_offset * (vertex & 1) ;

Davknapp · 2025-02-04T12:00:27Z

tutorials/general/t8_step4_partition_balance_ghost.cxx

@@ -208,7 +186,7 @@ t8_step4_main (int argc, char **argv)
  /* Initialize the sc library, has to happen before we initialize t8code. */
  sc_init (sc_MPI_COMM_WORLD, 1, 1, NULL, SC_LP_ESSENTIAL);
  /* Initialize t8code with log level SC_LP_PRODUCTION. See sc.h for more info on the log levels. */
-  t8_init (SC_LP_PRODUCTION);
+  t8_init (SC_LP_DEBUG);


Please change it back to SC_LP_PRODUCTION

Davknapp · 2025-02-04T12:40:20Z

tutorials/general/t8_step4_partition_balance_ghost.cxx

@@ -230,7 +208,7 @@ t8_step4_main (int argc, char **argv)
  t8_global_productionf (" [step4] Creating an adapted forest as in step3.\n");
  t8_global_productionf (" [step4] \n");
  /* Build a cube cmesh with tet, hex, and prism trees. */


Please udpate this commen.t

Davknapp · 2025-02-04T12:40:43Z

tutorials/general/t8_step4_partition_balance_ghost.cxx

+  forest = t8_forest_new_adapt (forest, t8_refine_first_child, 0, 0, NULL);
+  forest = t8_forest_new_adapt (forest, t8_refine_first_child, 0, 0, NULL);


Suggested change

forest = t8_forest_new_adapt (forest, t8_refine_first_child, 0, 0, NULL);

forest = t8_forest_new_adapt (forest, t8_refine_first_child, 0, 0, NULL);

forest = t8_forest_new_adapt (forest, t8_refine_first_child, 0, 0, NULL);

Davknapp · 2025-02-04T12:41:53Z

tutorials/general/t8_step4_partition_balance_ghost.cxx

- */
-static t8_forest_t
-t8_step4_balance (t8_forest_t forest)
+int


why is balance removed?
Maybe it is a good idea to make an exampe where one can compare vertex and face ghosts?

Because the changes in the tutorial were not meant to be reviewed or merged, rather to test stuff out locally.

This feature is not tested at all, and I see that as a bigger priority than fixing tutorials

lukasdreyer added 5 commits January 24, 2025 13:43

Add vertex functionality to scheme interface

2949bd6

Implement functionality for default quads

9a1c1d1

enable vertex ghosts

d6d69df

adapt tutorial to vertex ghost

5d85bb5

move debug print into T8_ENABLE_DEBUG

4218bec

Davknapp added priority: medium Should be solved within half a year workload: medium Would take a week or less labels Jan 27, 2025

Davknapp self-assigned this Jan 27, 2025

Davknapp requested changes Feb 4, 2025

View reviewed changes

Davknapp assigned lukasdreyer and unassigned Davknapp Feb 4, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature vertex ghosts #1353

Feature vertex ghosts #1353

lukasdreyer commented Jan 24, 2025

Davknapp left a comment

Davknapp Feb 4, 2025

Davknapp Feb 4, 2025

Davknapp Feb 4, 2025

Davknapp Feb 4, 2025

Davknapp Feb 4, 2025

Davknapp Feb 4, 2025

Davknapp Feb 4, 2025

Davknapp Feb 4, 2025

Davknapp Feb 4, 2025

Davknapp Feb 4, 2025

lukasdreyer Feb 4, 2025

	desc->x = elem->x + coord_offset * ((vertex & 1 << 0) >> 0);
	desc->x = elem->x + coord_offset * (vertex & 1) ;

		forest = t8_forest_new_adapt (forest, t8_refine_first_child, 0, 0, NULL);
		forest = t8_forest_new_adapt (forest, t8_refine_first_child, 0, 0, NULL);

Feature vertex ghosts #1353

Are you sure you want to change the base?

Feature vertex ghosts #1353

Conversation

lukasdreyer commented Jan 24, 2025

General

Tests

Github action

Scripts and Wiki

License

Tag Label

Davknapp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment