Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ghost interface #1278

Draft
wants to merge 37 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
d0a95a8
first ghost-interface draft
antjeHP Jun 24, 2024
4d72771
work status
antjeHP Jun 24, 2024
b8c6e37
update ghost_interface_unref, update set_ghost (not final), add debug…
antjeHP Jul 1, 2024
11db7f3
updated ghost_interface_face
antjeHP Jul 4, 2024
3786648
edit balance for ghost_interface
antjeHP Jul 4, 2024
cfa63f1
updated ghost_creat_ext to ghost_interface
antjeHP Jul 4, 2024
65fc4e4
delete unnecesssary prints
antjeHP Jul 4, 2024
c61a5a4
commed out the members ghost_type and ghost_algorithm in forest
antjeHP Jul 4, 2024
4bca580
fix ghost interface bug in blanace
antjeHP Aug 5, 2024
bf5add2
header for ghost interface
antjeHP Aug 5, 2024
d548aec
change includes for ghost interface
antjeHP Aug 5, 2024
b5ba195
update t8_forest_ghost_create_ext for ghost interface
antjeHP Aug 5, 2024
9b693d4
remove debuging print statmens
antjeHP Aug 5, 2024
dbfb924
add hypercube hybrid to benchmarks
antjeHP Aug 5, 2024
ce0a928
update Makefile for ghost_interface
antjeHP Aug 11, 2024
c47c536
fix benachmark bug
antjeHP Aug 11, 2024
a52d1f6
Merge remote-tracking branch 'origin/main' into feature-ghost_interface
antjeHP Aug 12, 2024
06fd2e2
add ghost_interface implemenation
antjeHP Aug 12, 2024
ba64acb
add ghost interface to makefile
antjeHP Aug 12, 2024
137c189
add more cubetests
antjeHP Aug 26, 2024
cd88e76
add ghost stencil
antjeHP Sep 9, 2024
0d74a11
update function name of step 2 of ghost interface
antjeHP Sep 9, 2024
53de48d
update ghost stencil
antjeHP Sep 11, 2024
febd5b1
add stencil find owner function
antjeHP Sep 16, 2024
c9bb0f4
typos
antjeHP Sep 30, 2024
5010b09
Detailed description of ghost interface functions
antjeHP Oct 7, 2024
39ca690
deleted unused file t8_forest_ghost_interface_faces.hxx
antjeHP Oct 23, 2024
b83d99a
Merge branch 'main' into feature-ghost_interface
antjeHP Oct 24, 2024
af2926a
remove unused variable in add_stencil
antjeHP Oct 29, 2024
68fb1e9
Apply suggestions from code review for const
antjeHP Nov 26, 2024
bea8572
assertion in t8_forest_set_ghost_ext_new
antjeHP Nov 26, 2024
11fbc50
updated year in copyright notice
antjeHP Nov 26, 2024
e93e2e9
add const
antjeHP Nov 26, 2024
bdbf825
update comments
antjeHP Dec 5, 2024
3582bc2
unused varable in realise
antjeHP Dec 5, 2024
a4d2b4e
add doc file
antjeHP Dec 5, 2024
b8c3a67
add more doc
antjeHP Dec 17, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 20 additions & 5 deletions benchmarks/time_forest_partition.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ main (int argc, char *argv[])
int level, level_diff;
int help = 0, no_vtk, do_ghost, do_balance, use_cad;
int dim, num_files;
int test_tet, test_linear_cylinder, test_cad_cylinder;
int test_tet, test_linear_cylinder, test_cad_cylinder, test_hybrid_cube, test_hex_cube;
int stride;
int cmesh_level;
double T, delta_t, cfl;
Expand Down Expand Up @@ -390,6 +390,12 @@ main (int argc, char *argv[])
sc_options_add_switch (opt, 'O', "test-cad-cylinder", &test_cad_cylinder,
"Use a cad cmesh to compare linear and cad geometry performance."
" If this option is used -o is enabled automatically. Not allowed with -f and -c.");
sc_options_add_switch (opt, 'C', "test-hybridcube", &test_hybrid_cube,
"Use a hypercube with Hybercube with Tets, Prism and Hex elements as cmesh."
" If this option is used -o is enabled automatically. Not allowed with -f and -c.");
sc_options_add_switch (opt, 'H', "test-hexcube", &test_hex_cube,
"Use a hypercube with Hex elements as cmesh."
" If this option is used -o is enabled automatically. Not allowed with -f and -c.");
sc_options_add_int (opt, 'l', "level", &level, 0, "The initial uniform refinement level of the forest.");
sc_options_add_int (opt, 'r', "rlevel", &level_diff, 1,
"The number of levels that the forest is refined from the initial level.");
Expand All @@ -415,11 +421,12 @@ main (int argc, char *argv[])
/* check for wrong usage of arguments */
if (first_argc < 0 || first_argc != argc || dim < 2 || dim > 3
|| (cmeshfileprefix == NULL && mshfileprefix == NULL && test_tet == 0 && test_cad_cylinder == 0
&& test_linear_cylinder == 0)
&& test_linear_cylinder == 0 && test_hybrid_cube == 0 && test_hex_cube == 0)
|| stride <= 0 || (num_files - 1) * stride >= mpisize || cfl < 0 || T <= 0
|| test_tet + test_linear_cylinder + test_cad_cylinder > 1
|| (cmesh_level >= 0 && (!test_linear_cylinder && !test_cad_cylinder))
|| ((mshfileprefix != NULL || cmeshfileprefix != NULL) && (test_linear_cylinder || test_cad_cylinder || test_tet))
|| test_tet + test_linear_cylinder + test_cad_cylinder + test_hybrid_cube + test_hex_cube > 1
|| (cmesh_level >= 0 && (!test_linear_cylinder && !test_cad_cylinder && !test_hybrid_cube && !test_hex_cube))
|| ((mshfileprefix != NULL || cmeshfileprefix != NULL)
&& (test_linear_cylinder || test_cad_cylinder || test_tet || test_hybrid_cube || test_hex_cube))
|| (mshfileprefix == NULL && use_cad)) {
sc_options_print_usage (t8_get_package_id (), SC_LP_ERROR, opt, NULL);
return 1;
Expand Down Expand Up @@ -454,6 +461,14 @@ main (int argc, char *argv[])
sc_intpow (2, cmesh_level), sc_intpow (2, cmesh_level), test_cad_cylinder);
test_linear_cylinder ? vtu_prefix = "test_linear_cylinder" : vtu_prefix = "test_cad_cylinder";
}
else if (test_hybrid_cube) {
cmesh = t8_cmesh_new_hypercube_hybrid (sc_MPI_COMM_WORLD, 0, 0);
vtu_prefix = "test_hypercube_hybrid";
}
else if (test_hex_cube) {
cmesh = t8_cmesh_new_hypercube (T8_ECLASS_HEX, sc_MPI_COMM_WORLD, 0, 0, 0);
vtu_prefix = "test_hypercube_hex";
}
else {
T8_ASSERT (cmeshfileprefix != NULL);
cmesh
Expand Down
7 changes: 6 additions & 1 deletion src/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,12 @@ libt8_installed_headers_forest = \
src/t8_forest/t8_forest_profiling.h \
src/t8_forest/t8_forest_io.h \
src/t8_forest/t8_forest_adapt.h \
src/t8_forest/t8_forest_iterate.h src/t8_forest/t8_forest_partition.h
src/t8_forest/t8_forest_iterate.h src/t8_forest/t8_forest_partition.h \
src/t8_forest/t8_forest_ghost_interface_wrapper.h \
src/t8_forest/t8_forest_ghost_interface.h \
src/t8_forest/t8_forest_ghost_interface.hxx \
src/t8_forest/t8_forest_ghost_search.hxx \
src/t8_forest/t8_forest_ghost_stencil.hxx
libt8_installed_headers_geometry = \
src/t8_geometry/t8_geometry.h \
src/t8_geometry/t8_geometry_handler.hxx \
Expand Down
44 changes: 29 additions & 15 deletions src/t8_forest/t8_forest.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include <t8_forest/t8_forest_partition.h>
#include <t8_forest/t8_forest_private.h>
#include <t8_forest/t8_forest_ghost.h>
#include <t8_forest/t8_forest_ghost_interface_wrapper.h>
#include <t8_forest/t8_forest_balance.h>
#include <t8_element.hxx>
#include <t8_element_c_interface.h>
Expand All @@ -39,6 +40,8 @@
#include <t8_forest/t8_forest_adapt.h>
#include <t8_vtk/t8_vtk_writer.h>
#include <t8_geometry/t8_geometry_base.hxx>
#include <t8_forest/t8_forest_ghost_interface.hxx>
#include <t8_forest/t8_forest_ghost_search.hxx>
#if T8_ENABLE_DEBUG
#include <t8_geometry/t8_geometry_implementations/t8_geometry_linear.h>
#include <t8_geometry/t8_geometry_implementations/t8_geometry_linear_axis_aligned.h>
Expand Down Expand Up @@ -2948,6 +2951,19 @@ t8_forest_set_balance (t8_forest_t forest, const t8_forest_t set_from, int no_re
}
}

void
t8_forest_set_ghost_ext_new (t8_forest_t forest, int do_ghost, t8_forest_ghost_interface_c *ghost_interface)
antjeHP marked this conversation as resolved.
Show resolved Hide resolved
{
T8_ASSERT (t8_forest_is_initialized (forest));
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");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator Author

@antjeHP antjeHP Nov 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

if (forest->ghost_interface != NULL) {
t8_forest_ghost_interface_unref (&(forest->ghost_interface));
}
forest->do_ghost = do_ghost;
forest->ghost_interface = ghost_interface;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Collaborator Author

@antjeHP antjeHP Nov 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe ghost_definition

}

void
t8_forest_set_ghost_ext (t8_forest_t forest, int do_ghost, t8_ghost_type_t ghost_type, int ghost_version)
{
Expand All @@ -2965,8 +2981,8 @@ t8_forest_set_ghost_ext (t8_forest_t forest, int do_ghost, t8_ghost_type_t ghost
forest->do_ghost = (do_ghost != 0); /* True if and only if do_ghost != 0 */
}
if (forest->do_ghost) {
forest->ghost_type = ghost_type;
forest->ghost_algorithm = ghost_version;
t8_forest_ghost_interface_c *ghost_interface = t8_forest_ghost_interface_face_new (ghost_version);
t8_forest_set_ghost_ext_new (forest, do_ghost, ghost_interface);
}
}

Expand Down Expand Up @@ -3230,6 +3246,12 @@ t8_forest_commit (t8_forest_t forest)
forest->scheme_cxx = forest->set_from->scheme_cxx;
forest->global_num_trees = forest->set_from->global_num_trees;

if (forest->ghost_interface == NULL && forest->set_from->ghost_interface != NULL) {
forest->ghost_interface = forest->set_from->ghost_interface;
t8_forest_ghost_interface_ref (forest->ghost_interface);
t8_debugf ("t8_forest_commit: uebernehme ghost von set_from\n");
antjeHP marked this conversation as resolved.
Show resolved Hide resolved
}

/* Compute the maximum allowed refinement level */
t8_forest_compute_maxlevel (forest);
if (forest->from_method == T8_FOREST_FROM_COPY) {
Expand Down Expand Up @@ -3383,19 +3405,7 @@ t8_forest_commit (t8_forest_t forest)
/* Construct a ghost layer, if desired */
if (forest->do_ghost) {
/* TODO: ghost type */
switch (forest->ghost_algorithm) {
case 1:
t8_forest_ghost_create_balanced_only (forest);
break;
case 2:
t8_forest_ghost_create (forest);
break;
case 3:
t8_forest_ghost_create_topdown (forest);
break;
default:
SC_ABORT ("Invalid choice of ghost algorithm");
}
t8_forest_ghost_create_ext (forest);
}
forest->do_ghost = 0;
}
Expand Down Expand Up @@ -4257,6 +4267,10 @@ t8_forest_reset (t8_forest_t *pforest)
if (forest->ghosts != NULL) {
t8_forest_ghost_unref (&forest->ghosts);
}
/* Destroy the ghost_interface class if it exist */
if (forest->ghost_interface != NULL) {
t8_forest_ghost_interface_unref (&(forest->ghost_interface));
}
/* we have taken ownership on calling t8_forest_set_* */
if (forest->scheme_cxx != NULL) {
t8_scheme_cxx_unref (&forest->scheme_cxx);
Expand Down
13 changes: 12 additions & 1 deletion src/t8_forest/t8_forest_balance.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
#include <t8_forest/t8_forest_types.h>
#include <t8_forest/t8_forest_private.h>
#include <t8_forest/t8_forest_ghost.h>
#include <t8_forest/t8_forest_ghost_interface.hxx>
#include <t8_forest/t8_forest_ghost_interface_wrapper.h>
#include <t8_forest/t8_forest_general.h>
#include <t8_forest/t8_forest_profiling.h>
#include <t8_element.hxx>
Expand Down Expand Up @@ -137,6 +139,7 @@ t8_forest_balance (t8_forest_t forest, int repartition)
int count_partition_stats = 0;
double ada_time, ghost_time, part_time;
sc_statinfo_t *adap_stats, *ghost_stats, *partition_stats;
int create_ghost_interface = 0;

t8_global_productionf ("Into t8_forest_balance with %lli global elements.\n",
(long long) t8_forest_get_global_num_elements (forest->set_from));
Expand Down Expand Up @@ -170,8 +173,16 @@ t8_forest_balance (t8_forest_t forest, int repartition)
t8_forest_ref (forest_from);

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;
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some comments would be nice

}

while (!done_global) {
Expand Down
22 changes: 18 additions & 4 deletions src/t8_forest/t8_forest_general.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
#include <t8_cmesh.h>
#include <t8_element.h>
#include <t8_data/t8_containers.h>
#include <t8_forest/t8_forest_ghost_interface.h>

/** Opaque pointer to a forest implementation. */
typedef struct t8_forest *t8_forest_t;
Expand All @@ -40,10 +41,11 @@ typedef struct t8_tree *t8_tree_t;
/** This type controls, which neighbors count as ghost elements.
* Currently, we support face-neighbors. Vertex and edge neighbors will eventually be added. */
typedef enum {
T8_GHOST_NONE = 0, /**< Do not create ghost layer. */
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_NONE = 0, /**< Do not create ghost layer. */
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*/
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

step1 and step2 were renamed

} t8_ghost_type_t;

/** This typedef is needed as a helper construct to
Expand Down Expand Up @@ -363,10 +365,22 @@ t8_forest_set_ghost (t8_forest_t forest, int do_ghost, t8_ghost_type_t ghost_typ
* If 2, the iterative algorithm for unbalanced forests.
* If 3, the top-down search algorithm for unbalanced forests.
* \see t8_forest_set_ghost
* \note this function creates an ghost_interface obcejct and call \ref t8_forest_set_ghost_ext_new
*/
void
t8_forest_set_ghost_ext (t8_forest_t forest, int do_ghost, t8_ghost_type_t ghost_type, int ghost_version);

/** Set a ghost_interface
* In application schoud only used if the user creates its own ghost_interface class (type = userderdefined)
* \param [in] forest the fores
antjeHP marked this conversation as resolved.
Show resolved Hide resolved
* \param [in] do_ghost ---
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Document

* \param [in] ghost_interface pointer to an object of the class ghost_interface or a derived class
* The forest takes ownership of the ghost_interface
* \note if the forest has already a ghost_interface, the old one will be unref and the new one will be set.
*/
void
t8_forest_set_ghost_ext_new (t8_forest_t forest, int do_ghost, t8_forest_ghost_interface_c *ghost_interface);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
t8_forest_set_ghost_ext_new (t8_forest_t forest, int do_ghost, t8_forest_ghost_interface_c *ghost_interface);
t8_forest_set_ghost_ext_new (t8_forest_t forest, const int do_ghost, const t8_forest_ghost_interface_c *ghost_interface);

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this function we set forest->ghost_interface = ghost_interface; and if I make the variable ghost_interface of the function const, I can't write it to the forest, because it expects no const member.


/* TODO: use assertions and document that the forest_set (..., from) and
* set_load are mutually exclusive. */
void
Expand Down
Loading
Loading