From 87f89fd87821ee69f1495bdb3422713a3c29d7eb Mon Sep 17 00:00:00 2001 From: "Dreyer, Lukas" Date: Tue, 18 Jun 2024 17:13:56 +0200 Subject: [PATCH 1/6] start fixing cmesh copy --- src/t8_cmesh/t8_cmesh_copy.c | 16 +++++++++------- src/t8_cmesh/t8_cmesh_trees.c | 1 + test/t8_cmesh/t8_gtest_cmesh_copy.cxx | 18 +++++------------- 3 files changed, 15 insertions(+), 20 deletions(-) diff --git a/src/t8_cmesh/t8_cmesh_copy.c b/src/t8_cmesh/t8_cmesh_copy.c index de5d0e7ea4..d0f0eeed0d 100644 --- a/src/t8_cmesh/t8_cmesh_copy.c +++ b/src/t8_cmesh/t8_cmesh_copy.c @@ -69,13 +69,15 @@ t8_cmesh_copy (t8_cmesh_t cmesh, t8_cmesh_t cmesh_from, sc_MPI_Comm comm) T8_ECLASS_COUNT * sizeof (t8_locidx_t)); /* Copy the tree info */ - num_parts = t8_cmesh_trees_get_numproc (cmesh_from->trees); cmesh->trees = NULL; - t8_cmesh_trees_init (&cmesh->trees, num_parts, cmesh_from->num_local_trees, cmesh_from->num_ghosts); - t8_cmesh_trees_copy_toproc (cmesh->trees, cmesh_from->trees, cmesh_from->num_local_trees, cmesh_from->num_ghosts); - for (iz = 0; iz < num_parts; iz++) { - t8_cmesh_trees_get_part_data (cmesh_from->trees, iz, &first_tree, &num_trees, &first_ghost, &num_ghosts); - t8_cmesh_trees_start_part (cmesh->trees, iz, first_tree, num_trees, first_ghost, num_ghosts, 0); - t8_cmesh_trees_copy_part (cmesh->trees, iz, cmesh_from->trees, iz); + if (cmesh_from->trees) { + num_parts = t8_cmesh_trees_get_numproc (cmesh_from->trees); + t8_cmesh_trees_init (&cmesh->trees, num_parts, cmesh_from->num_local_trees, cmesh_from->num_ghosts); + t8_cmesh_trees_copy_toproc (cmesh->trees, cmesh_from->trees, cmesh_from->num_local_trees, cmesh_from->num_ghosts); + for (iz = 0; iz < num_parts; iz++) { + t8_cmesh_trees_get_part_data (cmesh_from->trees, iz, &first_tree, &num_trees, &first_ghost, &num_ghosts); + t8_cmesh_trees_start_part (cmesh->trees, iz, first_tree, num_trees, first_ghost, num_ghosts, 0); + t8_cmesh_trees_copy_part (cmesh->trees, iz, cmesh_from->trees, iz); + } } } diff --git a/src/t8_cmesh/t8_cmesh_trees.c b/src/t8_cmesh/t8_cmesh_trees.c index 459bf24c35..d140ced7a5 100644 --- a/src/t8_cmesh/t8_cmesh_trees.c +++ b/src/t8_cmesh/t8_cmesh_trees.c @@ -791,6 +791,7 @@ t8_cmesh_trees_get_attribute (const t8_cmesh_trees_t trees, const t8_locidx_t lt size_t t8_cmesh_trees_get_numproc (const t8_cmesh_trees_t trees) { + T8_ASSERT (trees != NULL); return trees->from_proc->elem_count; } diff --git a/test/t8_cmesh/t8_gtest_cmesh_copy.cxx b/test/t8_cmesh/t8_gtest_cmesh_copy.cxx index 8b90426851..0002a7459c 100644 --- a/test/t8_cmesh/t8_gtest_cmesh_copy.cxx +++ b/test/t8_cmesh/t8_gtest_cmesh_copy.cxx @@ -34,18 +34,11 @@ * We test whether the new and original cmesh are equal. */ -/* Note: This test currently fails on many cmeshes and is thus deavtivated. - * See: https://github.com/DLR-AMR/t8code/issues/920 - */ - -/* Remove `DISABLED_` from the name of the Test(suite) or use `--gtest_also_run_disabled_tests` when you start working on the issue. */ -class DISABLED_t8_cmesh_copy: public testing::TestWithParam { +class t8_cmesh_copy: public testing::TestWithParam { protected: void SetUp () override { - /* Skip test since cmesh copy is not yet working. See https://github.com/DLR-AMR/t8code/issues/920 */ - cmesh_original = GetParam ()->cmesh_create (); /* Initialized test cmesh that we derive in the test */ @@ -55,8 +48,6 @@ class DISABLED_t8_cmesh_copy: public testing::TestWithParamtrees)) << "Cmesh face consistency failed."; } -TEST_P (DISABLED_t8_cmesh_copy, test_cmesh_copy) +TEST_P (t8_cmesh_copy, test_cmesh_copy) { + t8_cmesh_ref (cmesh_original); t8_cmesh_set_derive (cmesh, cmesh_original); t8_cmesh_commit (cmesh, sc_MPI_COMM_WORLD); test_cmesh_committed (cmesh); - EXPECT_TRUE (t8_cmesh_is_equal (cmesh, cmesh_original)); + t8_cmesh_unref (&cmesh_original); } /* Test all cmeshes over all different inputs*/ -INSTANTIATE_TEST_SUITE_P (t8_gtest_cmesh_copy, DISABLED_t8_cmesh_copy, AllCmeshsParam, pretty_print_base_example); +INSTANTIATE_TEST_SUITE_P (t8_gtest_cmesh_copy, t8_cmesh_copy, AllCmeshsParam, pretty_print_base_example); From f5fc47f1a5ee1e0e4f85ec1bbf1d2d44063498ee Mon Sep 17 00:00:00 2001 From: "Dreyer, Lukas" Date: Tue, 18 Jun 2024 17:59:22 +0200 Subject: [PATCH 2/6] Disable disjoint bricks for copy test --- test/t8_cmesh/t8_gtest_cmesh_copy.cxx | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/test/t8_cmesh/t8_gtest_cmesh_copy.cxx b/test/t8_cmesh/t8_gtest_cmesh_copy.cxx index 0002a7459c..3b2ed76b51 100644 --- a/test/t8_cmesh/t8_gtest_cmesh_copy.cxx +++ b/test/t8_cmesh/t8_gtest_cmesh_copy.cxx @@ -39,6 +39,14 @@ class t8_cmesh_copy: public testing::TestWithParam { void SetUp () override { + if (GetParam ()->name == std::string { "t8_cmesh_new_disjoint_brick_" }) { + GTEST_SKIP (); + } + else { + std::cout << "name:" << std::endl; + std::cout << GetParam ()->name << std::endl; + } + cmesh_original = GetParam ()->cmesh_create (); /* Initialized test cmesh that we derive in the test */ @@ -48,8 +56,10 @@ class t8_cmesh_copy: public testing::TestWithParam { void TearDown () override { - /* Unref both cmeshes */ - t8_cmesh_unref (&cmesh); + /* Unref cmesh, if test was not skipped */ + if (cmesh) { + t8_cmesh_unref (&cmesh); + } } t8_cmesh_t cmesh; From 94dcde33f981aee3c1065eacb0303c23a21455e6 Mon Sep 17 00:00:00 2001 From: "Dreyer, Lukas" Date: Wed, 26 Jun 2024 10:53:17 +0200 Subject: [PATCH 3/6] Actually memcpy the data, and not the container --- src/t8_cmesh/t8_cmesh_copy.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/t8_cmesh/t8_cmesh_copy.c b/src/t8_cmesh/t8_cmesh_copy.c index d0f0eeed0d..7046282fa9 100644 --- a/src/t8_cmesh/t8_cmesh_copy.c +++ b/src/t8_cmesh/t8_cmesh_copy.c @@ -60,7 +60,9 @@ t8_cmesh_copy (t8_cmesh_t cmesh, t8_cmesh_t cmesh_from, sc_MPI_Comm comm) if (cmesh_from->tree_offsets != NULL) { T8_ASSERT (cmesh->tree_offsets == NULL); cmesh->tree_offsets = t8_cmesh_alloc_offsets (cmesh->mpisize, comm); - sc_shmem_memcpy (cmesh->tree_offsets, cmesh_from->tree_offsets, sizeof (t8_gloidx_t) * (cmesh->mpisize + 1), comm); + sc_shmem_memcpy (t8_shmem_array_get_array (cmesh->tree_offsets), + t8_shmem_array_get_array (cmesh_from->tree_offsets), sizeof (t8_gloidx_t) * (cmesh->mpisize + 1), + comm); } /* Copy the numbers of trees */ memcpy (cmesh->num_trees_per_eclass, cmesh_from->num_trees_per_eclass, T8_ECLASS_COUNT * sizeof (t8_gloidx_t)); From b4ecf0689fd84ea42875e5ab1715649c673a7b8d Mon Sep 17 00:00:00 2001 From: "Dreyer, Lukas" Date: Wed, 26 Jun 2024 10:59:18 +0200 Subject: [PATCH 4/6] enable disjoint brick --- test/t8_cmesh/t8_gtest_cmesh_copy.cxx | 8 -------- 1 file changed, 8 deletions(-) diff --git a/test/t8_cmesh/t8_gtest_cmesh_copy.cxx b/test/t8_cmesh/t8_gtest_cmesh_copy.cxx index 3b2ed76b51..6379fcc396 100644 --- a/test/t8_cmesh/t8_gtest_cmesh_copy.cxx +++ b/test/t8_cmesh/t8_gtest_cmesh_copy.cxx @@ -39,14 +39,6 @@ class t8_cmesh_copy: public testing::TestWithParam { void SetUp () override { - if (GetParam ()->name == std::string { "t8_cmesh_new_disjoint_brick_" }) { - GTEST_SKIP (); - } - else { - std::cout << "name:" << std::endl; - std::cout << GetParam ()->name << std::endl; - } - cmesh_original = GetParam ()->cmesh_create (); /* Initialized test cmesh that we derive in the test */ From b2b25471b246dfedc086f36b61b717f6a00576ed Mon Sep 17 00:00:00 2001 From: "Dreyer, Lukas" Date: Wed, 26 Jun 2024 11:00:07 +0200 Subject: [PATCH 5/6] rename cmesh -> cmesh_copy and refactor rc management --- test/t8_cmesh/t8_gtest_cmesh_copy.cxx | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/test/t8_cmesh/t8_gtest_cmesh_copy.cxx b/test/t8_cmesh/t8_gtest_cmesh_copy.cxx index 6379fcc396..116ba9b937 100644 --- a/test/t8_cmesh/t8_gtest_cmesh_copy.cxx +++ b/test/t8_cmesh/t8_gtest_cmesh_copy.cxx @@ -40,21 +40,13 @@ class t8_cmesh_copy: public testing::TestWithParam { SetUp () override { cmesh_original = GetParam ()->cmesh_create (); - - /* Initialized test cmesh that we derive in the test */ - t8_cmesh_init (&cmesh); } void TearDown () override { - /* Unref cmesh, if test was not skipped */ - if (cmesh) { - t8_cmesh_unref (&cmesh); - } + t8_cmesh_unref (&cmesh_original); } - - t8_cmesh_t cmesh; t8_cmesh_t cmesh_original; }; @@ -67,15 +59,16 @@ test_cmesh_committed (t8_cmesh_t cmesh) TEST_P (t8_cmesh_copy, test_cmesh_copy) { + t8_cmesh_t cmesh_copy; + t8_cmesh_init (&cmesh_copy); t8_cmesh_ref (cmesh_original); - t8_cmesh_set_derive (cmesh, cmesh_original); - t8_cmesh_commit (cmesh, sc_MPI_COMM_WORLD); + t8_cmesh_set_derive (cmesh_copy, cmesh_original); + t8_cmesh_commit (cmesh_copy, sc_MPI_COMM_WORLD); - test_cmesh_committed (cmesh); - EXPECT_TRUE (t8_cmesh_is_equal (cmesh, cmesh_original)); - t8_cmesh_unref (&cmesh_original); + test_cmesh_committed (cmesh_copy); + EXPECT_TRUE (t8_cmesh_is_equal (cmesh_copy, cmesh_original)); + t8_cmesh_unref (&cmesh_copy); } /* Test all cmeshes over all different inputs*/ - INSTANTIATE_TEST_SUITE_P (t8_gtest_cmesh_copy, t8_cmesh_copy, AllCmeshsParam, pretty_print_base_example); From 70703c6abbc32321e869b5ada9bee3bf5195820c Mon Sep 17 00:00:00 2001 From: "Dreyer, Lukas" Date: Wed, 26 Jun 2024 11:12:03 +0200 Subject: [PATCH 6/6] Use correct wrapped shmem_array functionality to copy data --- src/t8_cmesh/t8_cmesh_copy.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/t8_cmesh/t8_cmesh_copy.c b/src/t8_cmesh/t8_cmesh_copy.c index 7046282fa9..69ddd98fd2 100644 --- a/src/t8_cmesh/t8_cmesh_copy.c +++ b/src/t8_cmesh/t8_cmesh_copy.c @@ -60,9 +60,7 @@ t8_cmesh_copy (t8_cmesh_t cmesh, t8_cmesh_t cmesh_from, sc_MPI_Comm comm) if (cmesh_from->tree_offsets != NULL) { T8_ASSERT (cmesh->tree_offsets == NULL); cmesh->tree_offsets = t8_cmesh_alloc_offsets (cmesh->mpisize, comm); - sc_shmem_memcpy (t8_shmem_array_get_array (cmesh->tree_offsets), - t8_shmem_array_get_array (cmesh_from->tree_offsets), sizeof (t8_gloidx_t) * (cmesh->mpisize + 1), - comm); + t8_shmem_array_copy (cmesh->tree_offsets, cmesh_from->tree_offsets); } /* Copy the numbers of trees */ memcpy (cmesh->num_trees_per_eclass, cmesh_from->num_trees_per_eclass, T8_ECLASS_COUNT * sizeof (t8_gloidx_t));