VIC icon indicating copy to clipboard operation
VIC copied to clipboard

ENH: Better validation of MPI_Datatypes

Open jhamman opened this issue 9 years ago • 3 comments

I’m trying to think of a more robust validation approach when creating the MPI_Datatypes in vic_mpi_support.c. Right now, we check that the number of members in the MPI struct matches the number of members we counted in the actual structure definition (most likely in vic_def.h). So we are currently checking that we have the right number of members. This is prone to errors because I can easily add a member to a structure in vic_def.h and not add it in vic_mpi_support.c. Would it be better/possible to check that the size of the MPI_Datatype matches the size of the original structure? In other words sizeof(options_struct) == sum(sizeof(mpi_members))?

jhamman avatar Apr 01 '16 03:04 jhamman

I tried to implement this (see below for an example, by tracking the size of the member elements. Unfortunately, sizeof(struct) does not necessarily equal the sum of its members, because the structures add padding to align on n-byte boundaries (for example, 4 byte boundaries). The sizeof() includes this padding. Implementing this would likely be hardware dependent, so I have taken it back out, but I’ll keep working on this.

The first function works because the structure aligns on 4-byte boundaries. The second does not work, and the logic I added does not fix it (I understand why). To be continued ...

/******************************************************************************
 * @brief   Create an MPI_Datatype that represents the filenames_struct
 * @details This allows MPI operations in which the entire filenames_struct
 *          can be treated as an MPI_Datatype. NOTE: This function needs to be
 *          kept in-sync with the global_param_struct data type in vic_def.h.
 *
 * @param mpi_type MPI_Datatype that can be used in MPI operations
 *****************************************************************************/
void
create_MPI_filenames_struct_type(MPI_Datatype *mpi_type)
{
    int           nitems; // number of elements in struct
    int           status;
    int          *blocklengths;
    int           mpi_size; // size in bytes of MPI data type
    size_t        i;
    size_t        ssize; // sum of all elements - should equal sizeof(struct)
    MPI_Aint     *offsets;
    MPI_Datatype *mpi_types;

    // nitems has to equal the number of elements in filenames_struct
    nitems = 14;
    blocklengths = malloc(nitems * sizeof(*blocklengths));
    if (blocklengths == NULL) {
        log_err("Memory allocation error in create_MPI_filenames_struct_type().")
    }

    offsets = malloc(nitems * sizeof(*offsets));
    if (offsets == NULL) {
        log_err("Memory allocation error in create_MPI_filenames_struct_type().")
    }

    mpi_types = malloc(nitems * sizeof(*mpi_types));
    if (mpi_types == NULL) {
        log_err("Memory allocation error in create_MPI_filenames_struct_type().")
    }

    // most of the elements in filenames_struct are character arrays. Use
    // MAXSTRING as the default block length and reset as needed
    for (i = 0; i < (size_t) nitems; i++) {
        blocklengths[i] = MAXSTRING;
    }

    // reset i
    i = 0;

    // initialize ssize
    ssize = 0;

    // char forcing[2][MAXSTRING];
    offsets[i] = offsetof(filenames_struct, forcing);
    blocklengths[i] *= 2;
    mpi_types[i] = MPI_CHAR;
    MPI_Type_size(mpi_types[i], &mpi_size);
    ssize += mpi_size * blocklengths[i];
    i += 1;

    // char f_path_pfx[2][MAXSTRING];
    offsets[i] = offsetof(filenames_struct, f_path_pfx);
    blocklengths[i] *= 2;
    mpi_types[i] = MPI_CHAR;
    MPI_Type_size(mpi_types[i], &mpi_size);
    ssize += mpi_size * blocklengths[i];
    i += 1;

    // char global[MAXSTRING];
    offsets[i] = offsetof(filenames_struct, global);
    mpi_types[i] = MPI_CHAR;
    MPI_Type_size(mpi_types[i], &mpi_size);
    ssize += mpi_size * blocklengths[i];
    i += 1;

    // char domain[MAXSTRING];
    offsets[i] = offsetof(filenames_struct, domain);
    mpi_types[i] = MPI_CHAR;
    MPI_Type_size(mpi_types[i], &mpi_size);
    ssize += mpi_size * blocklengths[i];
    i += 1;

    // char constants[MAXSTRING];
    offsets[i] = offsetof(filenames_struct, constants);
    mpi_types[i] = MPI_CHAR;
    MPI_Type_size(mpi_types[i], &mpi_size);
    ssize += mpi_size * blocklengths[i];
    i += 1;

    // char init_state[MAXSTRING];
    offsets[i] = offsetof(filenames_struct, init_state);
    mpi_types[i] = MPI_CHAR;
    MPI_Type_size(mpi_types[i], &mpi_size);
    ssize += mpi_size * blocklengths[i];
    i += 1;

    // char lakeparam[MAXSTRING];
    offsets[i] = offsetof(filenames_struct, lakeparam);
    mpi_types[i] = MPI_CHAR;
    MPI_Type_size(mpi_types[i], &mpi_size);
    ssize += mpi_size * blocklengths[i];
    i += 1;

    // char result_dir[MAXSTRING];
    offsets[i] = offsetof(filenames_struct, result_dir);
    mpi_types[i] = MPI_CHAR;
    MPI_Type_size(mpi_types[i], &mpi_size);
    ssize += mpi_size * blocklengths[i];
    i += 1;

    // char snowband[MAXSTRING];
    offsets[i] = offsetof(filenames_struct, snowband);
    mpi_types[i] = MPI_CHAR;
    MPI_Type_size(mpi_types[i], &mpi_size);
    ssize += mpi_size * blocklengths[i];
    i += 1;

    // char soil[MAXSTRING];
    offsets[i] = offsetof(filenames_struct, soil);
    mpi_types[i] = MPI_CHAR;
    MPI_Type_size(mpi_types[i], &mpi_size);
    ssize += mpi_size * blocklengths[i];
    i += 1;

    // char statefile[MAXSTRING];
    offsets[i] = offsetof(filenames_struct, statefile);
    mpi_types[i] = MPI_CHAR;
    MPI_Type_size(mpi_types[i], &mpi_size);
    ssize += mpi_size * blocklengths[i];
    i += 1;

    // char veg[MAXSTRING];
    offsets[i] = offsetof(filenames_struct, veg);
    mpi_types[i] = MPI_CHAR;
    MPI_Type_size(mpi_types[i], &mpi_size);
    ssize += mpi_size * blocklengths[i];
    i += 1;

    // char veglib[MAXSTRING];
    offsets[i] = offsetof(filenames_struct, veglib);
    mpi_types[i] = MPI_CHAR;
    MPI_Type_size(mpi_types[i], &mpi_size);
    ssize += mpi_size * blocklengths[i];
    i += 1;

    // char log_path[MAXSTRING];
    offsets[i] = offsetof(filenames_struct, log_path);
    mpi_types[i] = MPI_CHAR;
    MPI_Type_size(mpi_types[i], &mpi_size);
    ssize += mpi_size * blocklengths[i];
    i += 1;


    // make sure that the we have the right number of elements
    if (i != (size_t) nitems) {
        log_err("Miscount in create_MPI_filenames_struct_type(): "
                "%zd not equal to %d\n", i, nitems);
    }

    // make sure that we have the right size
    if (ssize != sizeof(filenames_struct)) {
      log_err("Incorrect size in create_MPI_filenames_struct_type(): "
              "%zd not equal to %zd\n", ssize, sizeof(filenames_struct));
    }

    status = MPI_Type_create_struct(nitems, blocklengths, offsets, mpi_types,
                                    mpi_type);
    if (status != MPI_SUCCESS) {
        log_err("MPI error in create_MPI_filenames_struct_type(): %d\n",
                status);
    }

    status = MPI_Type_commit(mpi_type);
    if (status != MPI_SUCCESS) {
        log_err("MPI error in create_MPI_filenames_struct_type(): %d\n",
                status);
    }

    // cleanup
    free(blocklengths);
    free(offsets);
    free(mpi_types);
}

/******************************************************************************
 * @brief   Create an MPI_Datatype that represents the location_struct
 * @details This allows MPI operations in which the entire location_struct
 *          can be treated as an MPI_Datatype. NOTE: This function needs to be
 *          kept in-sync with the location_struct data type in
 *          vic_image_driver.h.
 *
 * @param mpi_type MPI_Datatype that can be used in MPI operations
 *****************************************************************************/
void
create_MPI_location_struct_type(MPI_Datatype *mpi_type)
{
    int           nitems; // number of elements in struct
    int           status;
    int          *blocklengths;
    int           mpi_size; // size in bytes of MPI data type
    size_t        i;
    size_t        ssize; // sum of all elements - should equal sizeof(struct)
    MPI_Aint     *offsets;
    MPI_Datatype *mpi_types;

    // nitems has to equal the number of elements in global_param_struct
    nitems = 9;
    blocklengths = malloc(nitems * sizeof(*blocklengths));
    if (blocklengths == NULL) {
        log_err("Memory allocation error in create_MPI_location_struct_type().")
    }

    offsets = malloc(nitems * sizeof(*offsets));
    if (offsets == NULL) {
        log_err("Memory allocation error in create_MPI_location_struct_type().")
    }

    mpi_types = malloc(nitems * sizeof(*mpi_types));
    if (mpi_types == NULL) {
        log_err("Memory allocation error in create_MPI_location_struct_type().")
    }

    // none of the elements in location_struct are arrays.
    for (i = 0; i < (size_t) nitems; i++) {
        blocklengths[i] = 1;
    }

    // reset i
    i = 0;

    // initialize ssize
    ssize = 0;

    // size_t run;
    offsets[i] = offsetof(location_struct, run);
    mpi_types[i] = MPI_C_BOOL;
    MPI_Type_size(mpi_types[i], &mpi_size);
    ssize += mpi_size * blocklengths[i];
    if (ssize % ALIGN_BYTES) {
      ssize = (ssize / ALIGN_BYTES + 1) * ALIGN_BYTES;
    }
    i += 1;

    // double latitude;
    offsets[i] = offsetof(location_struct, latitude);
    mpi_types[i] = MPI_DOUBLE;
    MPI_Type_size(mpi_types[i], &mpi_size);
    ssize += mpi_size * blocklengths[i];
    i += 1;

    // double longitude;
    offsets[i] = offsetof(location_struct, longitude);
    mpi_types[i] = MPI_DOUBLE;
    MPI_Type_size(mpi_types[i], &mpi_size);
    ssize += mpi_size * blocklengths[i];
    i += 1;

    // double area;
    offsets[i] = offsetof(location_struct, area);
    mpi_types[i] = MPI_DOUBLE;
    MPI_Type_size(mpi_types[i], &mpi_size);
    ssize += mpi_size * blocklengths[i];
    i += 1;

    // double frac;
    offsets[i] = offsetof(location_struct, frac);
    mpi_types[i] = MPI_DOUBLE;
    MPI_Type_size(mpi_types[i], &mpi_size);
    ssize += mpi_size * blocklengths[i];
    i += 1;

    // size_t nveg;
    offsets[i] = offsetof(location_struct, nveg);
    mpi_types[i] = MPI_AINT;
    MPI_Type_size(mpi_types[i], &mpi_size);
    ssize += mpi_size * blocklengths[i];
    i += 1;

    // size_t global_idx;
    offsets[i] = offsetof(location_struct, global_idx);
    mpi_types[i] = MPI_AINT;
    MPI_Type_size(mpi_types[i], &mpi_size);
    ssize += mpi_size * blocklengths[i];
    i += 1;

    // size_t io_idx;
    offsets[i] = offsetof(location_struct, io_idx);
    mpi_types[i] = MPI_AINT;
    MPI_Type_size(mpi_types[i], &mpi_size);
    ssize += mpi_size * blocklengths[i];
    i += 1;

    // size_t local_idx;
    offsets[i] = offsetof(location_struct, local_idx);
    mpi_types[i] = MPI_AINT;
    MPI_Type_size(mpi_types[i], &mpi_size);
    ssize += mpi_size * blocklengths[i];
    i += 1;

    // make sure that the we have the right number of elements
    if (i != (size_t) nitems) {
        log_err("Miscount in create_MPI_location_struct_type(): "
                "%zd not equal to %d\n", i, nitems);
    }

    // make sure that we have the right size
    if (ssize != sizeof(location_struct)) {
      log_err("Incorrect size in create_MPI_location_struct_type(): "
              "%zd not equal to %zd\n", ssize, sizeof(location_struct));
    }

    status = MPI_Type_create_struct(nitems, blocklengths, offsets, mpi_types,
                                    mpi_type);
    if (status != MPI_SUCCESS) {
        log_err("MPI error in create_MPI_location_struct_type(): %d\n", status);
    }

    status = MPI_Type_commit(mpi_type);
    if (status != MPI_SUCCESS) {
        log_err("MPI error in create_MPI_location_struct_type(): %d\n", status);
    }

    // cleanup
    free(blocklengths);
    free(offsets);
    free(mpi_types);
}

bartnijssen avatar Apr 01 '16 06:04 bartnijssen

@jhamman : Also note that vic_mpi_support.c has a built in test routine that we should automatically run as part of travis tests. If we keep this updated then it should automatically capture problems like the one we have seen here (or at least I think so, I still need to check that).

bartnijssen avatar Apr 01 '16 07:04 bartnijssen

The test in vic_mpi_support() needs work since it does not capture the error that we saw in issue #445 and which was solved with PR #450

bartnijssen avatar Apr 02 '16 18:04 bartnijssen