t8code icon indicating copy to clipboard operation
t8code copied to clipboard

Cmesh_new_hypercube does not work with partition flag

Open holke opened this issue 4 years ago • 5 comments

It seems that the vertex coordinates are not partitioned correctly.

To reproduce:

 cmesh = t8_cmesh_new_hypercube (T8_ECLASS_TET, comm, 0, 1, 0);
 t8_cmesh_vtk_write_file (cmesh, "vtk_filename", 1.0);

holke avatar Mar 19 '21 19:03 holke

This bug was also verified for the cmesh_new_from_p{4,8}est functions. The example/cmesh/t8_cmesh_partition example does not work anymore.

holke avatar Nov 17 '21 16:11 holke

First thing that I stumbled upon:

The function t8_cmesh_add_attributes is called by t8_cmesh_commit_replicated_new and t8_cmesh_commit_partitioned_new, so for me it looks like the comment

      /* attribute->id is a gloidx that is casted to a locidx here.
       * Should not cause problems, since mesh is replicated */

from below might not be true.

void
t8_cmesh_add_attributes (t8_cmesh_t cmesh)
{
  t8_stash_attribute_struct_t *attribute;
  t8_stash_t          stash = cmesh->stash;
  t8_locidx_t         ltree;
  size_t              si, sj;

  ltree = -1;
  for (si = 0, sj = 0; si < stash->attributes.elem_count; si++, sj++) {
    attribute = (t8_stash_attribute_struct_t *)
      sc_array_index (&stash->attributes, si);
    if (cmesh->first_tree <= attribute->id &&
        attribute->id < cmesh->first_tree + cmesh->num_local_trees) {
      if (attribute->id > ltree) {
        /* Enter a new tree */
        ltree = attribute->id;
        sj = 0;
      }
      /* attribute->id is a gloidx that is casted to a locidx here.
       * Should not cause problems, since mesh is replicated */
      T8_ASSERT (attribute->id - cmesh->first_tree ==
                 (t8_locidx_t) attribute->id - cmesh->first_tree);
      t8_cmesh_trees_add_attribute (cmesh->trees, 0, attribute,
                                    attribute->id - cmesh->first_tree, sj);
    }
  }
}

I will try to gain a deeper understanding about the different kinds of local and global indices from the wiki page.

lukasdreyer avatar Jan 19 '23 12:01 lukasdreyer

I think I need a highlevel overview of the capabilities that t8_cmesh_commit_partitioned_new should have. From my current understanding, the problem arises, because the attributes of ghost trees are not partitioned when a cmesh is constructed partitioned from stash.

You can test this by changing

int
t8_cmesh_vtk_write_file (t8_cmesh_t cmesh, const char *fileprefix,
                         double scale)
{
  T8_ASSERT (scale == 1.0);
  return t8_cmesh_vtk_write_file_ext (cmesh, fileprefix, 1.0, 1);
}

to

int
t8_cmesh_vtk_write_file (t8_cmesh_t cmesh, const char *fileprefix,
                         double scale)
{
  T8_ASSERT (scale == 1.0);
  return t8_cmesh_vtk_write_file_ext (cmesh, fileprefix, 1.0, 0);
}

i.e. telling the vtk funktion to not write the ghost information. This lets the example above pass.

lukasdreyer avatar Jan 19 '23 15:01 lukasdreyer

Casting from global to local would only result in a problem when we have an overflow. So when the mesh has more global trees than fit into a locidx_t (2^31 - 1 = 2.147.483.647). And i doubt this is the case for your example ;)

holke avatar Jan 19 '23 16:01 holke

The first post was more of an observation, that it seemed to me that some functionality, that was originally planned for the replicated case, was also used for the partitioned case. From my understanding, the calculation

attribute->id - cmesh->first_tree

transforms the global treeid attribute->id to a local treeid, but this simple subtraction only works for local trees.

As the example problem crashes, because the ghost trees don't have the vertices set as attributes (see second post), my conclusion is that the ghost tree attributes do not get set. Now my question is if I overlooked the place where the ghost tree attributes should be set, or if they should also be set by this function and I misinterpreted what it is doing.

lukasdreyer avatar Jan 20 '23 08:01 lukasdreyer

I think this was fixed by #448. The following example looks good to me on 2 and 6 procs:

#include <t8_cmesh_vtk_writer.h>
#include <t8_cmesh/t8_cmesh_partition.h>
#include <t8_cmesh/t8_cmesh_examples.h>


int
main (int argc, char **argv)
{
  int mpiret;

  mpiret = sc_MPI_Init (&argc, &argv);
  SC_CHECK_MPI (mpiret);

  sc_init (sc_MPI_COMM_WORLD, 1, 1, NULL, SC_LP_ESSENTIAL);
  t8_init (SC_LP_DEFAULT);

  t8_cmesh_t cmesh = t8_cmesh_new_hypercube (T8_ECLASS_TET, sc_MPI_COMM_WORLD, 0, 1, 0);
  t8_cmesh_vtk_write_file (cmesh, "vtk_filename");
  t8_cmesh_destroy(&cmesh);
  sc_finalize ();

  mpiret = sc_MPI_Finalize ();
  SC_CHECK_MPI (mpiret);
  return 0;
}

@holke and @Niklas997, please reopen if you disagree.

lukasdreyer avatar Jun 18 '24 09:06 lukasdreyer