Ghost interface #1278

antjeHP · 2024-10-24T13:35:48Z

Describe your changes here:

Design and implementation of the feature Ghost Interface.
Creating a class design for a Ghost interface with abstract base class and derived classes for specific ghost-types (Definitions of neighborhoods).
The type of ghost-algorithm (befor only face-neighborhoods supported), was stored in the forest struct until yet.
Now the forest struct stores a pointer to a Ghost-Interface class.
Not finished yet

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)

…ing prints

sandro-elsweijer · 2024-10-29T10:08:33Z

src/t8_forest/t8_forest.cxx

+  SC_CHECK_ABORT (do_ghost != 0, "do_ghost == 0 in set_ghost_ext_new.\n");
+  SC_CHECK_ABORT (ghost_interface != NULL, "invalides ghost interface in set_ghost_ext_new\n");


This negates the possibility to deactivate ghost. One could want to call

t8_forest_set_ghost_ext_new (forest, false, NULL)

to deactivate ghost during a specific part of the simulation.
IMHO

T8_ASSERT (!(do_ghost == 1 && ghost_interface == NULL);

would be a better check

I see your point. But a little question, why should we use T8_ASSERT instead of SC_CHECK_ABORT ?
If someone sets the combination 1 and NULL, this can not work.

In the function t8_forest_set_ghost_ext we also use SC_CHECK_ABORT, that was the reason for me to do the same.

I see your point. But a little question, why should we use T8_ASSERT instead of SC_CHECK_ABORT ? If someone sets the combination 1 and NULL, this can not work.

In the function t8_forest_set_ghost_ext we also use SC_CHECK_ABORT, that was the reason for me to do the same.

We want to use T8_ASSERT as often as possible, because it does not affect the performance in release mode. In this case a SC_CHECK_ABORT is also okay, because this function is not called that often

src/t8_forest/t8_forest.cxx

sandro-elsweijer · 2024-10-29T10:21:20Z

src/t8_forest/t8_forest_balance.cxx

  if (forest->set_from->ghosts == NULL) {
-    forest->set_from->ghost_type = T8_GHOST_FACES;
+    // forest->set_from->ghost_type = T8_GHOST_FACES;
+    if (forest->set_from->ghost_interface == NULL) {
+      forest->set_from->ghost_interface = t8_forest_ghost_interface_face_new (3);
+      create_ghost_interface = 1;
+    }
    t8_forest_ghost_create_topdown (forest->set_from);
+    if (create_ghost_interface) {
+      t8_forest_ghost_interface_unref (&(forest->set_from->ghost_interface));
+      forest->set_from->ghost_interface = NULL;
+    }


Some comments would be nice

sandro-elsweijer · 2024-10-29T10:22:00Z

src/t8_forest/t8_forest_general.h

+  T8_GHOST_FACES,      /**< Consider all face (codimension 1) neighbors. */
+  T8_GHOST_EDGES,      /**< Consider all edge (codimension 2) and face neighbors. */
+  T8_GHOST_VERTICES,   /**< Consider all vertex (codimension 3) and edge and face neighbors. */
+  T8_GHOST_USERDEFINED /** If the user define by his self a step_1 or step_2 function*/


step1 and step2 were renamed

sandro-elsweijer · 2024-10-29T10:26:34Z

src/t8_forest/t8_forest.cxx

+    t8_forest_ghost_interface_unref (&(forest->ghost_interface));
+  }
+  forest->do_ghost = do_ghost;
+  forest->ghost_interface = ghost_interface;


Maybe we can find a more suitable name. Yes, this is an interface, but in this case the class defines what the ghost algorithm actually does. Maybe ghost_strategy or ghost_recipe?

Maybe ghost_creator or ghost_builder is a good name. Because the class contains all what is necessary to build the ghost layer and the only function, that you can call from outside is do_ghost.
If we want to take multiple ghost_algorithms on one forest, the forest will save the handler for them and not longer the ghost_interface class.

I see where you are going there, but these names suggest that the class builds another object like the t8_scheme_builder.

maybe ghost_definition

src/t8_forest/t8_forest_ghost_stencil.hxx

src/t8_forest/t8_forest_ghost_search.hxx

src/t8_forest/t8_forest_ghost_stencil.hxx

sandro-elsweijer · 2024-10-29T14:44:30Z

src/t8_forest/t8_forest_types.h

+  // t8_ghost_type_t ghost_type;     /**< If a ghost layer will be created, the type of neighbors that count as ghost. */
+  // int ghost_algorithm;            /**< Controls the algorithm used for ghost. 1 = balanced only. 2 = also unbalanced
+  //  3 = top-down search and unbalanced. */


Is this still needed?

No, this is no longer necessary. The ghost_type and the ghost_algorithm are part of the class.

sandro-elsweijer · 2024-10-29T14:49:07Z

Please also create an author file for your work under t8code/doc/

Co-authored-by: Sandro Elsweijer <[email protected]>

sandro-elsweijer · 2024-12-09T14:48:20Z

src/t8_forest/t8_forest_ghost.cxx

+#ifdef T8_ENABLE_DEBUG
+#endif


Probably there once was something between this, but it got removed and somebody forgot to remove this

sandro-elsweijer · 2024-12-10T13:44:13Z

src/t8_forest/t8_forest.cxx

+    t8_forest_ghost_interface_unref (&(forest->ghost_interface));
+  }
+  forest->do_ghost = do_ghost;
+  forest->ghost_interface = ghost_interface;


maybe ghost_definition

sandro-elsweijer · 2024-12-19T09:16:44Z

src/t8_forest/t8_forest_ghost_interface_wrapper.h

+
+/**
+ * Satisfy the C interface of forest
+ * Return for a ghost_interface of Type FACE the ghost_algorithm / ghost_version (1, 2 or 3)


Yes you can use \note

antjeHP added 28 commits June 24, 2024 15:16

first ghost-interface draft

d0a95a8

work status

4d72771

update ghost_interface_unref, update set_ghost (not final), add debug…

b8c6e37

…ing prints

updated ghost_interface_face

11db7f3

edit balance for ghost_interface

3786648

updated ghost_creat_ext to ghost_interface

cfa63f1

delete unnecesssary prints

65fc4e4

commed out the members ghost_type and ghost_algorithm in forest

c61a5a4

fix ghost interface bug in blanace

4bca580

header for ghost interface

bf5add2

change includes for ghost interface

d548aec

update t8_forest_ghost_create_ext for ghost interface

b5ba195

remove debuging print statmens

9b693d4

add hypercube hybrid to benchmarks

dbfb924

update Makefile for ghost_interface

ce0a928

fix benachmark bug

c47c536

Merge remote-tracking branch 'origin/main' into feature-ghost_interface

a52d1f6

add ghost_interface implemenation

06fd2e2

add ghost interface to makefile

ba64acb

add more cubetests

137c189

add ghost stencil

cd88e76

update function name of step 2 of ghost interface

0d74a11

update ghost stencil

53de48d

add stencil find owner function

febd5b1

typos

c9bb0f4

Detailed description of ghost interface functions

5010b09

deleted unused file t8_forest_ghost_interface_faces.hxx

39ca690

Merge branch 'main' into feature-ghost_interface

b83d99a

antjeHP added New feature Adds a new feature to the code draft Enhance the visibility that this is a draft. labels Oct 24, 2024

antjeHP added priority: medium Should be solved within half a year workload: medium Would take a week or less labels Oct 24, 2024

antjeHP self-assigned this Oct 24, 2024

remove unused variable in add_stencil

af2926a

sandro-elsweijer requested changes Oct 29, 2024

View reviewed changes

antjeHP assigned sandro-elsweijer and antjeHP and unassigned antjeHP and sandro-elsweijer Nov 8, 2024

antjeHP and others added 7 commits November 26, 2024 13:37

Apply suggestions from code review for const

68fb1e9

Co-authored-by: Sandro Elsweijer <[email protected]>

assertion in t8_forest_set_ghost_ext_new

bea8572

updated year in copyright notice

11fbc50

add const

e93e2e9

update comments

bdbf825

unused varable in realise

3582bc2

add doc file

a4d2b4e

antjeHP assigned sandro-elsweijer and unassigned antjeHP Dec 5, 2024

add more doc

b8c3a67

antjeHP force-pushed the feature-ghost_interface branch from edd08d5 to b8c3a67 Compare December 17, 2024 12:45

sandro-elsweijer requested changes Dec 19, 2024

View reviewed changes

sandro-elsweijer assigned antjeHP and unassigned sandro-elsweijer Dec 19, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ghost interface #1278

Ghost interface #1278

antjeHP commented Oct 24, 2024 •

edited

Loading

sandro-elsweijer Oct 29, 2024

antjeHP Nov 26, 2024 •

edited

Loading

sandro-elsweijer Dec 9, 2024

sandro-elsweijer Oct 29, 2024

sandro-elsweijer Oct 29, 2024

sandro-elsweijer Oct 29, 2024

antjeHP Nov 26, 2024 •

edited

Loading

sandro-elsweijer Dec 9, 2024

sandro-elsweijer Dec 10, 2024

sandro-elsweijer Oct 29, 2024

antjeHP Nov 26, 2024

sandro-elsweijer commented Oct 29, 2024

sandro-elsweijer Dec 9, 2024

sandro-elsweijer Dec 10, 2024

sandro-elsweijer Dec 19, 2024

		SC_CHECK_ABORT (do_ghost != 0, "do_ghost == 0 in set_ghost_ext_new.\n");
		SC_CHECK_ABORT (ghost_interface != NULL, "invalides ghost interface in set_ghost_ext_new\n");

		#ifdef T8_ENABLE_DEBUG
		#endif

Ghost interface #1278

Are you sure you want to change the base?

Ghost interface #1278

Conversation

antjeHP commented Oct 24, 2024 • edited Loading

General

Tests

Github action

Scripts and Wiki

License

Choose a reason for hiding this comment

antjeHP Nov 26, 2024 • edited Loading

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

antjeHP Nov 26, 2024 • edited Loading

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

sandro-elsweijer commented Oct 29, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

antjeHP commented Oct 24, 2024 •

edited

Loading

antjeHP Nov 26, 2024 •

edited

Loading

antjeHP Nov 26, 2024 •

edited

Loading