From b29971f2bd26368dfad0e23da983116a182854d2 Mon Sep 17 00:00:00 2001 From: Jan-Willem Blokland Date: Mon, 11 Sep 2023 14:43:32 +0200 Subject: [PATCH 1/3] rest_vol_dataset: (fix) Memory-side hyperslab - Improved the implementation of memory-side hyperslab when writing data by taking into account if the selected data is contiguous and starting memory offset. - Fixed various hyperslab and point tests. - Implemented a test in which the memory-side hyperslab is non-contiguous in memory. - Implemented a test in which the memory-side hyperslab is contiguous in memory but with a non-zero offset in memory. --- src/rest_vol_dataset.c | 190 +++++++++++++++++++++++++- test/test_rest_vol.c | 293 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 473 insertions(+), 10 deletions(-) diff --git a/src/rest_vol_dataset.c b/src/rest_vol_dataset.c index e0e94410..2764323c 100644 --- a/src/rest_vol_dataset.c +++ b/src/rest_vol_dataset.c @@ -35,11 +35,15 @@ static herr_t RV_convert_dataset_creation_properties_to_JSON(hid_t dcpl_id, char static herr_t RV_convert_dataspace_selection_to_string(hid_t space_id, char **selection_string, size_t *selection_string_len, hbool_t req_param); +/* Helper function for dataspace selection */ +static htri_t RV_dataspace_selection_is_contiguous(hid_t space_id); + /* Conversion function to convert one or more rest_obj_ref_t objects into a binary buffer for data transfer */ -static herr_t RV_convert_obj_refs_to_buffer(const rv_obj_ref_t *ref_array, size_t ref_array_len, - char **buf_out, size_t *buf_out_len); -static herr_t RV_convert_buffer_to_obj_refs(char *ref_buf, size_t ref_buf_len, rv_obj_ref_t **buf_out, - size_t *buf_out_len); +static herr_t RV_convert_obj_refs_to_buffer(const rv_obj_ref_t *ref_array, size_t ref_array_len, + char **buf_out, size_t *buf_out_len); +static herr_t RV_convert_buffer_to_obj_refs(char *ref_buf, size_t ref_buf_len, rv_obj_ref_t **buf_out, + size_t *buf_out_len); +static hssize_t RV_convert_start_to_offset(hid_t space_id); /* Callbacks used for post-processing after a curl request succeeds */ static herr_t rv_dataset_read_cb(hid_t mem_type_id, hid_t mem_space_id, void *buf, @@ -804,9 +808,11 @@ RV_dataset_write(size_t count, void *dset[], hid_t mem_type_id[], hid_t _mem_spa H5S_sel_type sel_type = H5S_SEL_ALL; H5T_class_t dtype_class; hbool_t is_transfer_binary = FALSE; + htri_t contiguous = FALSE; htri_t is_variable_str; hssize_t mem_select_npoints = 0; hssize_t file_select_npoints = 0; + hssize_t offset = 0; size_t host_header_len = 0; size_t write_body_len = 0; size_t selection_body_len = 0; @@ -982,6 +988,25 @@ RV_dataset_write(size_t count, void *dset[], hid_t mem_type_id[], hid_t _mem_spa FUNC_GOTO_ERROR(H5E_DATATYPE, H5E_BADVALUE, FAIL, "memory datatype is invalid"); write_body_len = (size_t)file_select_npoints * dtype_size; + if ((contiguous = RV_dataspace_selection_is_contiguous(transfer_info[i].mem_space_id)) < 0) + FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_BADVALUE, FAIL, + "Unable to determine if the dataspace selection is contiguous"); + if (!contiguous) { + if (NULL == (transfer_info[i].u.write_info.write_body = (char *)RV_malloc(write_body_len))) + FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_CANTALLOC, FAIL, + "can't allocate space for the 'write_body' values"); + if (H5Dgather(transfer_info[i].mem_space_id, buf[i], transfer_info[i].mem_type_id, + write_body_len, transfer_info[i].u.write_info.write_body, NULL, + transfer_info[i].u.write_info.write_body) < 0) + FUNC_GOTO_ERROR(H5E_DATASET, H5E_WRITEERROR, FAIL, "can't gather data to write buffer"); + buf[i] = transfer_info[i].u.write_info.write_body; + } + else { + if ((offset = RV_convert_start_to_offset(transfer_info[i].mem_space_id)) < 0) + FUNC_GOTO_ERROR(H5E_DATASET, H5E_BADVALUE, FAIL, + "Unable to determine memory offset value"); + buf[i] = buf[i] + offset * dtype_size; + } } /* end if */ else { if (H5T_STD_REF_OBJ == transfer_info[i].mem_type_id) { @@ -1060,6 +1085,8 @@ RV_dataset_write(size_t count, void *dset[], hid_t mem_type_id[], hid_t _mem_spa transfer_info[i].u.write_info.base64_encoded_values); #endif + if (transfer_info[i].u.write_info.write_body) + RV_free(transfer_info[i].u.write_info.write_body); write_body_len = (strlen(fmt_string) - 4) + selection_body_len + value_body_len; if (NULL == (transfer_info[i].u.write_info.write_body = RV_malloc(write_body_len + 1))) FUNC_GOTO_ERROR(H5E_DATASET, H5E_CANTALLOC, FAIL, "can't allocate space for write buffer"); @@ -4137,4 +4164,157 @@ rv_dataset_write_cb(hid_t mem_type_id, hid_t mem_space_id, void *buf, struct res { herr_t ret_value = SUCCEED; return SUCCEED; -} \ No newline at end of file +} + +/*------------------------------------------------------------------------- + * Function: RV_dataspace_selection_is_contiguous + * + * Purpose: Checks if the specified dataspace in a contiguous selection. + * + * Return: TRUE or FALSE if the selection is contiguous or + * non-contiguous and FAIL if it is unable to determine it. + * + * Programmer: Jan-Willem Blokland + * August, 2023 + */ +static htri_t +RV_dataspace_selection_is_contiguous(hid_t space_id) +{ + htri_t ret_value = TRUE; + hbool_t whole = TRUE; + hsize_t *dims = NULL; + hsize_t *start = NULL; + hsize_t *stride = NULL; + hsize_t *count = NULL; + hsize_t *block = NULL; + int i; + int ndims; + hssize_t npoints, nblocks; + + if ((npoints = H5Sget_select_npoints(space_id)) < 0) + FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_CANTGET, FAIL, "can't retrieve number of selected points"); + if (npoints < 2) + FUNC_GOTO_DONE(TRUE); + + if ((ndims = H5Sget_simple_extent_ndims(space_id)) < 0) + FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_CANTGET, FAIL, "can't retrieve dataspace dimensionality"); + if (!ndims) + FUNC_GOTO_DONE(TRUE); + + if (H5S_SEL_HYPERSLABS == H5Sget_select_type(space_id)) { + if (NULL == (dims = (hsize_t *)RV_malloc((size_t)ndims * sizeof(*dims)))) + FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_CANTALLOC, FAIL, + "can't allocate space for dimension 'dims' values"); + + if (H5Sget_simple_extent_dims(space_id, dims, NULL) < 0) + FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_CANTGET, FAIL, "can't get dataspace dimension size"); + + if (NULL == (start = (hsize_t *)RV_malloc((size_t)ndims * sizeof(*start)))) + FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_CANTALLOC, FAIL, + "can't allocate space for hyperslab selection 'start' values"); + if (NULL == (stride = (hsize_t *)RV_malloc((size_t)ndims * sizeof(*stride)))) + FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_CANTALLOC, FAIL, + "can't allocate space for hyperslab selection 'stride' values"); + if (NULL == (count = (hsize_t *)RV_malloc((size_t)ndims * sizeof(*count)))) + FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_CANTALLOC, FAIL, + "can't allocate space for hyperslab selection 'count' values"); + if (NULL == (block = (hsize_t *)RV_malloc((size_t)ndims * sizeof(*block)))) + FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_CANTALLOC, FAIL, + "can't allocate space for hyperslab selection 'block' values"); + + if (nblocks = H5Sget_select_hyper_nblocks(space_id) < 0) + FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_CANTGET, FAIL, "can't get number of hyperslab blocks"); + + if (H5Sget_regular_hyperslab(space_id, start, stride, count, block) < 0) + FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_CANTGET, FAIL, "can't get regular hyperslab selection"); + + /* For contiguous, the stride should be 1. */ + for (i = 0; i < ndims; i++) { + if (stride[i] > 1) + FUNC_GOTO_DONE(FALSE); + } + + if (nblocks > 1) { + /* Multiple blocks: count should be 1 except for the last dimension (fastest) */ + for (i = 0; i < ndims - 1; i++) { + if (count[i] > 1) + FUNC_GOTO_DONE(FALSE); + } + } + + /* For contiguous, all faster running dimensions than the current dimension should be selected + * completely */ + whole = (start[ndims - 1] == 0) && (count[ndims - 1] * block[ndims - 1] == dims[ndims - 1]); + for (i = ndims - 2; i >= 0; i--) { + if ((dims[i] > 1) && (count[i] * block[i] > 1) && !whole) + FUNC_GOTO_DONE(FALSE); + + whole = whole && (start[i] == 0) && (count[i] * block[i] == dims[i]); + } + } /* end if */ + +done: + if (block) + RV_free(block); + if (count) + RV_free(count); + if (dims) + RV_free(dims); + if (stride) + RV_free(stride); + if (start) + RV_free(start); + + return ret_value; +} /* end RV_dataspace_selection_is_contiguous() */ + +/*------------------------------------------------------------------------- + * Function: RV_convert_start_to_offset + * + * Purpose: Convert starting position value to an offset value. + * + * Return: Offset value on success/Negative value on failure. + * + * Programmer: Jan-Willem Blokland + * August, 2023 + */ +static hssize_t +RV_convert_start_to_offset(hid_t space_id) +{ + hsize_t *dims = NULL; + hsize_t *start = NULL; + hssize_t ret_value = 0; + int ndims, i; + + if (H5S_SEL_HYPERSLABS == H5Sget_select_type(space_id)) { + if ((ndims = H5Sget_simple_extent_ndims(space_id)) < 0) + FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_CANTGET, -1, "can't retrieve dataspace dimensionality"); + + if (NULL == (dims = (hsize_t *)RV_malloc((size_t)ndims * sizeof(*dims)))) + FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_CANTALLOC, -1, + "can't allocate space for dimension 'dims' values"); + + if (H5Sget_simple_extent_dims(space_id, dims, NULL) < 0) + FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_CANTGET, -1, "can't get dataspace dimension size"); + + if (NULL == (start = (hsize_t *)RV_malloc((size_t)ndims * sizeof(*start)))) + FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_CANTALLOC, -1, + "can't allocate space for hyperslab selection 'start' values"); + + if (H5Sget_regular_hyperslab(space_id, start, NULL, NULL, NULL) < 0) + FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_CANTGET, -1, "can't get regular hyperslab selection"); + + ret_value = start[0]; + for (i = 1; i < ndims; i++) { + ret_value = ret_value * dims[i] + start[i]; + } + } + +done: + if (dims) + RV_free(dims); + if (start) + RV_free(start); + + return ret_value; +} /* end RV_convert_start_to_offset() */ diff --git a/test/test_rest_vol.c b/test/test_rest_vol.c index d9a9ca65..9d53c991 100644 --- a/test/test_rest_vol.c +++ b/test/test_rest_vol.c @@ -7478,11 +7478,13 @@ test_write_dataset_small_point_selection(void) hsize_t points[DATASET_SMALL_WRITE_TEST_POINT_SELECTION_NUM_POINTS * DATASET_SMALL_WRITE_TEST_POINT_SELECTION_DSET_SPACE_RANK]; hsize_t dims[DATASET_SMALL_WRITE_TEST_POINT_SELECTION_DSET_SPACE_RANK] = {10, 10, 10}; + hsize_t mdims[1]; size_t i, data_size; hid_t file_id = -1, fapl_id = -1; hid_t container_group = -1; hid_t dset_id = -1; hid_t fspace_id = -1; + hid_t mspace_id = -1; void *data = NULL; TESTING("small write to dataset w/ point selection") @@ -7525,6 +7527,9 @@ test_write_dataset_small_point_selection(void) if (NULL == (data = malloc(data_size))) TEST_ERROR + mdims[0] = DATASET_SMALL_WRITE_TEST_POINT_SELECTION_NUM_POINTS; + if ((mspace_id = H5Screate_simple(1, mdims, NULL)) < 0) + TEST_ERROR for (i = 0; i < data_size / DATASET_SMALL_WRITE_TEST_POINT_SELECTION_DSET_DTYPESIZE; i++) ((int *)data)[i] = (int)i; @@ -7546,7 +7551,7 @@ test_write_dataset_small_point_selection(void) puts("Writing a small amount of data to dataset using a point selection\n"); #endif - if (H5Dwrite(dset_id, DATASET_SMALL_WRITE_TEST_POINT_SELECTION_DSET_DTYPE, H5S_ALL, fspace_id, + if (H5Dwrite(dset_id, DATASET_SMALL_WRITE_TEST_POINT_SELECTION_DSET_DTYPE, mspace_id, fspace_id, H5P_DEFAULT, data) < 0) { H5_FAILED(); printf(" couldn't write to dataset\n"); @@ -7558,6 +7563,8 @@ test_write_dataset_small_point_selection(void) data = NULL; } + if (H5Sclose(mspace_id) < 0) + TEST_ERROR if (H5Sclose(fspace_id) < 0) TEST_ERROR if (H5Dclose(dset_id) < 0) @@ -8553,6 +8560,7 @@ test_write_dataset_data_verification(void) { hssize_t space_npoints; hsize_t dims[DATASET_DATA_VERIFY_WRITE_TEST_DSET_SPACE_RANK] = {10, 10, 10}; + hsize_t mdims[2]; hsize_t start[DATASET_DATA_VERIFY_WRITE_TEST_DSET_SPACE_RANK]; hsize_t stride[DATASET_DATA_VERIFY_WRITE_TEST_DSET_SPACE_RANK]; hsize_t count[DATASET_DATA_VERIFY_WRITE_TEST_DSET_SPACE_RANK]; @@ -8564,6 +8572,7 @@ test_write_dataset_data_verification(void) hid_t container_group = -1; hid_t dset_id = -1; hid_t fspace_id = -1; + hid_t mspace_id = -1; void *data = NULL; void *write_buf = NULL; void *read_buf = NULL; @@ -8678,7 +8687,7 @@ test_write_dataset_data_verification(void) } #ifdef RV_CONNECTOR_DEBUG - puts("Writing to dataset using hyperslab selection\n"); + puts("Writing to dataset using hyperslab selection - contiguous\n"); #endif data_size = dims[1] * 2 * DATASET_DATA_VERIFY_WRITE_TEST_DSET_DTYPESIZE; @@ -8711,23 +8720,159 @@ test_write_dataset_data_verification(void) } /* Write to first two rows of dataset */ + mdims[0] = dims[1] * 2; + start[0] = 0; + stride[0] = 1; + count[0] = dims[1] * 2; + block[0] = 1; + if ((mspace_id = H5Screate_simple(1, mdims, NULL)) < 0) + TEST_ERROR + if (H5Sselect_hyperslab(mspace_id, H5S_SELECT_SET, start, stride, count, block) < 0) + TEST_ERROR + start[0] = start[1] = start[2] = 0; stride[0] = stride[1] = stride[2] = 1; count[0] = 2; count[1] = dims[1]; count[2] = 1; block[0] = block[1] = block[2] = 1; + if (H5Sselect_hyperslab(fspace_id, H5S_SELECT_SET, start, stride, count, block) < 0) + TEST_ERROR + + if (H5Dwrite(dset_id, DATASET_DATA_VERIFY_WRITE_TEST_DSET_DTYPE, mspace_id, fspace_id, H5P_DEFAULT, + write_buf) < 0) { + H5_FAILED(); + printf(" couldn't write to dataset\n"); + goto error; + } + + if (H5Sclose(mspace_id) < 0) + TEST_ERROR + if (H5Sclose(fspace_id) < 0) + TEST_ERROR + if (H5Dclose(dset_id) < 0) + TEST_ERROR + + if ((dset_id = H5Dopen2(file_id, "/" DATASET_TEST_GROUP_NAME "/" DATASET_DATA_VERIFY_WRITE_TEST_DSET_NAME, + H5P_DEFAULT)) < 0) { + H5_FAILED(); + printf(" couldn't open dataset\n"); + goto error; + } + if ((fspace_id = H5Dget_space(dset_id)) < 0) { + H5_FAILED(); + printf(" couldn't get dataset dataspace\n"); + goto error; + } + + if ((space_npoints = H5Sget_simple_extent_npoints(fspace_id)) < 0) { + H5_FAILED(); + printf(" couldn't get dataspace num points\n"); + goto error; + } + + if (NULL == (read_buf = malloc((hsize_t)space_npoints * DATASET_DATA_VERIFY_WRITE_TEST_DSET_DTYPESIZE))) + TEST_ERROR + +#ifdef RV_CONNECTOR_DEBUG + puts("Verifying that the data that comes back is correct after writing to the dataset using a hyperslab " + "selection\n"); +#endif + + if (H5Dread(dset_id, DATASET_DATA_VERIFY_WRITE_TEST_DSET_DTYPE, H5S_ALL, H5S_ALL, H5P_DEFAULT, read_buf) < + 0) { + H5_FAILED(); + printf(" couldn't read from dataset\n"); + goto error; + } + + if (memcmp(data, read_buf, data_size)) { + H5_FAILED(); + printf(" hyperslab selection data (contiguous) verification failed\n"); + goto error; + } + + if (data) { + free(data); + data = NULL; + } + + if (write_buf) { + free(write_buf); + write_buf = NULL; + } + + if (read_buf) { + free(read_buf); + read_buf = NULL; + } + +#ifdef RV_CONNECTOR_DEBUG + puts("Writing to dataset using hyperslab selection - contiguous - non-zero offset\n"); +#endif + + data_size = dims[1] * 2 * DATASET_DATA_VERIFY_WRITE_TEST_DSET_DTYPESIZE; + + if (NULL == (write_buf = malloc(data_size))) + TEST_ERROR + + for (i = 0; i < dims[1]; i++) + ((int *)write_buf)[i] = 68; + for (i = dims[1]; i < data_size / DATASET_DATA_VERIFY_WRITE_TEST_DSET_DTYPESIZE; i++) + ((int *)write_buf)[i] = 67; + + for (i = 0, data_size = 1; i < DATASET_DATA_VERIFY_WRITE_TEST_DSET_SPACE_RANK; i++) + data_size *= dims[i]; + data_size *= DATASET_DATA_VERIFY_WRITE_TEST_DSET_DTYPESIZE; + + if (NULL == (data = malloc(data_size))) + TEST_ERROR + + if (H5Dread(dset_id, DATASET_DATA_VERIFY_WRITE_TEST_DSET_DTYPE, H5S_ALL, H5S_ALL, H5P_DEFAULT, data) < + 0) { + H5_FAILED(); + printf(" couldn't read from dataset\n"); + goto error; + } + + for (i = 2; i < 3; i++) { + size_t j; + + for (j = 0; j < dims[1]; j++) + ((int *)data)[(i * dims[1] * dims[2]) + (j * dims[2])] = 67; + } + + /* Write to third row of dataset */ + mdims[0] = dims[1] * 2; + start[0] = dims[1]; + stride[0] = 1; + count[0] = dims[1]; + block[0] = 1; + if ((mspace_id = H5Screate_simple(1, mdims, NULL)) < 0) + TEST_ERROR + if (H5Sselect_hyperslab(mspace_id, H5S_SELECT_SET, start, stride, count, block) < 0) + TEST_ERROR + + start[0] = 2; + start[1] = start[2] = 0; + stride[0] = stride[1] = stride[2] = 1; + count[0] = 1; + count[1] = dims[1]; + count[2] = 1; + block[0] = block[1] = block[2] = 1; if (H5Sselect_hyperslab(fspace_id, H5S_SELECT_SET, start, stride, count, block) < 0) TEST_ERROR - if (H5Dwrite(dset_id, DATASET_DATA_VERIFY_WRITE_TEST_DSET_DTYPE, H5S_ALL, fspace_id, H5P_DEFAULT, + if (H5Dwrite(dset_id, DATASET_DATA_VERIFY_WRITE_TEST_DSET_DTYPE, mspace_id, fspace_id, H5P_DEFAULT, write_buf) < 0) { H5_FAILED(); printf(" couldn't write to dataset\n"); goto error; } + if (H5Sclose(mspace_id) < 0) + TEST_ERROR if (H5Sclose(fspace_id) < 0) TEST_ERROR if (H5Dclose(dset_id) < 0) @@ -8769,7 +8914,134 @@ test_write_dataset_data_verification(void) if (memcmp(data, read_buf, data_size)) { H5_FAILED(); - printf(" hyperslab selection data verification failed\n"); + printf(" hyperslab selection data (contiguous) verification failed\n"); + goto error; + } + + if (data) { + free(data); + data = NULL; + } + + if (write_buf) { + free(write_buf); + write_buf = NULL; + } + + if (read_buf) { + free(read_buf); + read_buf = NULL; + } + +#ifdef RV_CONNECTOR_DEBUG + puts("Writing to dataset using hyperslab selection - non-contiguous\n"); +#endif + + data_size = dims[1] * 2 * DATASET_DATA_VERIFY_WRITE_TEST_DSET_DTYPESIZE; + + if (NULL == (write_buf = malloc(data_size))) + TEST_ERROR + + for (i = 0; i < data_size / DATASET_DATA_VERIFY_WRITE_TEST_DSET_DTYPESIZE; i = i + 2) + ((int *)write_buf)[i] = 78; + for (i = 1; i < data_size / DATASET_DATA_VERIFY_WRITE_TEST_DSET_DTYPESIZE; i = i + 2) + ((int *)write_buf)[i] = 79; + + for (i = 0, data_size = 1; i < DATASET_DATA_VERIFY_WRITE_TEST_DSET_SPACE_RANK; i++) + data_size *= dims[i]; + data_size *= DATASET_DATA_VERIFY_WRITE_TEST_DSET_DTYPESIZE; + + if (NULL == (data = malloc(data_size))) + TEST_ERROR + + if (H5Dread(dset_id, DATASET_DATA_VERIFY_WRITE_TEST_DSET_DTYPE, H5S_ALL, H5S_ALL, H5P_DEFAULT, data) < + 0) { + H5_FAILED(); + printf(" couldn't read from dataset\n"); + goto error; + } + + for (i = 3; i < 4; i++) { + size_t j; + + for (j = 0; j < dims[1]; j++) + ((int *)data)[(i * dims[1] * dims[2]) + (j * dims[2])] = 78; + } + + /* Write to fourth row of dataset */ + mdims[0] = dims[1]; + mdims[1] = 2; + start[0] = start[1] = 0; + stride[0] = stride[1] = 1; + count[0] = dims[1]; + count[1] = 1; + block[0] = block[1] = 1; + if ((mspace_id = H5Screate_simple(2, mdims, NULL)) < 0) + TEST_ERROR + if (H5Sselect_hyperslab(mspace_id, H5S_SELECT_SET, start, stride, count, block) < 0) + TEST_ERROR + + start[0] = 3; + start[1] = start[2] = 0; + stride[0] = stride[1] = stride[2] = 1; + count[0] = 1; + count[1] = dims[1]; + count[2] = 1; + block[0] = block[1] = block[2] = 1; + if (H5Sselect_hyperslab(fspace_id, H5S_SELECT_SET, start, stride, count, block) < 0) + TEST_ERROR + + if (H5Dwrite(dset_id, DATASET_DATA_VERIFY_WRITE_TEST_DSET_DTYPE, mspace_id, fspace_id, H5P_DEFAULT, + write_buf) < 0) { + H5_FAILED(); + printf(" couldn't write to dataset\n"); + goto error; + } + + if (H5Sclose(mspace_id) < 0) + TEST_ERROR + if (H5Sclose(fspace_id) < 0) + TEST_ERROR + if (H5Dclose(dset_id) < 0) + TEST_ERROR + + if ((dset_id = H5Dopen2(file_id, "/" DATASET_TEST_GROUP_NAME "/" DATASET_DATA_VERIFY_WRITE_TEST_DSET_NAME, + H5P_DEFAULT)) < 0) { + H5_FAILED(); + printf(" couldn't open dataset\n"); + goto error; + } + + if ((fspace_id = H5Dget_space(dset_id)) < 0) { + H5_FAILED(); + printf(" couldn't get dataset dataspace\n"); + goto error; + } + + if ((space_npoints = H5Sget_simple_extent_npoints(fspace_id)) < 0) { + H5_FAILED(); + printf(" couldn't get dataspace num points\n"); + goto error; + } + + if (NULL == (read_buf = malloc((hsize_t)space_npoints * DATASET_DATA_VERIFY_WRITE_TEST_DSET_DTYPESIZE))) + TEST_ERROR + +#ifdef RV_CONNECTOR_DEBUG + puts("Verifying that the data that comes back is correct after writing to the dataset using a hyperslab " + "selection\n"); +#endif + + if (H5Dread(dset_id, DATASET_DATA_VERIFY_WRITE_TEST_DSET_DTYPE, H5S_ALL, H5S_ALL, H5P_DEFAULT, read_buf) < + 0) { + H5_FAILED(); + printf(" couldn't read from dataset\n"); + goto error; + } + + if (memcmp(data, read_buf, data_size)) { + H5_FAILED(); + printf(" hyperslab selection data (non-contiguous) verification failed\n"); goto error; } @@ -8828,6 +9100,15 @@ test_write_dataset_data_verification(void) } /* Select a series of 10 points in the dataset */ + mdims[0] = DATASET_DATA_VERIFY_WRITE_TEST_NUM_POINTS; + if ((mspace_id = H5Screate_simple(1, mdims, NULL)) < 0) + TEST_ERROR + for (i = 0; i < DATASET_DATA_VERIFY_WRITE_TEST_NUM_POINTS; i++) { + points[i] = i; + } + if (H5Sselect_elements(mspace_id, H5S_SELECT_SET, DATASET_DATA_VERIFY_WRITE_TEST_NUM_POINTS, points) < 0) + TEST_ERROR + for (i = 0; i < DATASET_DATA_VERIFY_WRITE_TEST_NUM_POINTS; i++) { size_t j; @@ -8838,13 +9119,15 @@ test_write_dataset_data_verification(void) if (H5Sselect_elements(fspace_id, H5S_SELECT_SET, DATASET_DATA_VERIFY_WRITE_TEST_NUM_POINTS, points) < 0) TEST_ERROR - if (H5Dwrite(dset_id, DATASET_DATA_VERIFY_WRITE_TEST_DSET_DTYPE, H5S_ALL, fspace_id, H5P_DEFAULT, + if (H5Dwrite(dset_id, DATASET_DATA_VERIFY_WRITE_TEST_DSET_DTYPE, mspace_id, fspace_id, H5P_DEFAULT, write_buf) < 0) { H5_FAILED(); printf(" couldn't write to dataset\n"); goto error; } + if (H5Sclose(mspace_id) < 0) + TEST_ERROR if (H5Sclose(fspace_id) < 0) TEST_ERROR if (H5Dclose(dset_id) < 0) From 3267575ee5c03ef552d6763154af85d52d7a7fc6 Mon Sep 17 00:00:00 2001 From: Jan-Willem Blokland Date: Tue, 12 Sep 2023 08:02:39 +0200 Subject: [PATCH 2/3] rest_vol_dataset: (fix) Point selection - Implement the assumption that any point selection is non-contiguous in memory in both RV_dataspace_selection_is_contiguous() and RV_convert_start_to_offset() function. --- src/rest_vol_dataset.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/rest_vol_dataset.c b/src/rest_vol_dataset.c index 2764323c..73bd3100 100644 --- a/src/rest_vol_dataset.c +++ b/src/rest_vol_dataset.c @@ -870,7 +870,7 @@ RV_dataset_write(size_t count, void *dset[], hid_t mem_type_id[], hid_t _mem_spa transfer_info[i].u.write_info.write_body = NULL; transfer_info[i].u.write_info.base64_encoded_values = NULL; transfer_info[i].dataset = (RV_object_t *)dset[i]; - transfer_info[i].buf = buf[i]; + transfer_info[i].buf = (void *)buf[i]; transfer_info[i].transfer_type = WRITE; transfer_info[i].mem_space_id = _mem_space_id[i]; @@ -4251,7 +4251,11 @@ RV_dataspace_selection_is_contiguous(hid_t space_id) whole = whole && (start[i] == 0) && (count[i] * block[i] == dims[i]); } - } /* end if */ + } /* end if*/ + else if (H5S_SEL_POINTS == H5Sget_select_type(space_id)) { + /* Assumption: any point selection is non-contiguous in memory */ + FUNC_GOTO_DONE(FALSE); + } /* end else if */ done: if (block) @@ -4308,7 +4312,11 @@ RV_convert_start_to_offset(hid_t space_id) for (i = 1; i < ndims; i++) { ret_value = ret_value * dims[i] + start[i]; } - } + } /* end if */ + else if (H5S_SEL_POINTS == H5Sget_select_type(space_id)) { + FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_BADVALUE, -1, + "for point selection, computing the offset is not supported"); + } /* end else if */ done: if (dims) From f22aa9cb3137defa9d9f883a866ea6bd943f12dd Mon Sep 17 00:00:00 2001 From: Jan-Willem Blokland Date: Thu, 14 Sep 2023 08:36:48 +0200 Subject: [PATCH 3/3] rest_vol_dataset: (fix) Memory-side hyperslab - The function RV_dataspace_is_contiguous() will return a non-regular hyperslab as non-contiguous in memory. - Generalized the function RV_convert_start_to_offset() such it works for both regular and non-regular hyperslab. - For completeness handle the other selection types explicitly in the function RV_dataspace_is_contiguous() and RV_convert_start_to_offset(). --- src/rest_vol_dataset.c | 189 ++++++++++++++++++++++++----------------- 1 file changed, 113 insertions(+), 76 deletions(-) diff --git a/src/rest_vol_dataset.c b/src/rest_vol_dataset.c index 73bd3100..9d4f1b59 100644 --- a/src/rest_vol_dataset.c +++ b/src/rest_vol_dataset.c @@ -996,8 +996,7 @@ RV_dataset_write(size_t count, void *dset[], hid_t mem_type_id[], hid_t _mem_spa FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_CANTALLOC, FAIL, "can't allocate space for the 'write_body' values"); if (H5Dgather(transfer_info[i].mem_space_id, buf[i], transfer_info[i].mem_type_id, - write_body_len, transfer_info[i].u.write_info.write_body, NULL, - transfer_info[i].u.write_info.write_body) < 0) + write_body_len, transfer_info[i].u.write_info.write_body, NULL, NULL) < 0) FUNC_GOTO_ERROR(H5E_DATASET, H5E_WRITEERROR, FAIL, "can't gather data to write buffer"); buf[i] = transfer_info[i].u.write_info.write_body; } @@ -1085,8 +1084,10 @@ RV_dataset_write(size_t count, void *dset[], hid_t mem_type_id[], hid_t _mem_spa transfer_info[i].u.write_info.base64_encoded_values); #endif - if (transfer_info[i].u.write_info.write_body) + if (transfer_info[i].u.write_info.write_body) { RV_free(transfer_info[i].u.write_info.write_body); + transfer_info[i].u.write_info.write_body = NULL; + } write_body_len = (strlen(fmt_string) - 4) + selection_body_len + value_body_len; if (NULL == (transfer_info[i].u.write_info.write_body = RV_malloc(write_body_len + 1))) FUNC_GOTO_ERROR(H5E_DATASET, H5E_CANTALLOC, FAIL, "can't allocate space for write buffer"); @@ -4182,6 +4183,7 @@ RV_dataspace_selection_is_contiguous(hid_t space_id) { htri_t ret_value = TRUE; hbool_t whole = TRUE; + htri_t regular = TRUE; hsize_t *dims = NULL; hsize_t *start = NULL; hsize_t *stride = NULL; @@ -4192,70 +4194,87 @@ RV_dataspace_selection_is_contiguous(hid_t space_id) hssize_t npoints, nblocks; if ((npoints = H5Sget_select_npoints(space_id)) < 0) - FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_CANTGET, FAIL, "can't retrieve number of selected points"); + FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_CANTGET, FAIL, "can't get number of selected points"); if (npoints < 2) FUNC_GOTO_DONE(TRUE); if ((ndims = H5Sget_simple_extent_ndims(space_id)) < 0) - FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_CANTGET, FAIL, "can't retrieve dataspace dimensionality"); + FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_CANTGET, FAIL, "can't get dataspace dimensionality"); if (!ndims) FUNC_GOTO_DONE(TRUE); - if (H5S_SEL_HYPERSLABS == H5Sget_select_type(space_id)) { - if (NULL == (dims = (hsize_t *)RV_malloc((size_t)ndims * sizeof(*dims)))) - FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_CANTALLOC, FAIL, - "can't allocate space for dimension 'dims' values"); - - if (H5Sget_simple_extent_dims(space_id, dims, NULL) < 0) - FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_CANTGET, FAIL, "can't get dataspace dimension size"); - - if (NULL == (start = (hsize_t *)RV_malloc((size_t)ndims * sizeof(*start)))) - FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_CANTALLOC, FAIL, - "can't allocate space for hyperslab selection 'start' values"); - if (NULL == (stride = (hsize_t *)RV_malloc((size_t)ndims * sizeof(*stride)))) - FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_CANTALLOC, FAIL, - "can't allocate space for hyperslab selection 'stride' values"); - if (NULL == (count = (hsize_t *)RV_malloc((size_t)ndims * sizeof(*count)))) - FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_CANTALLOC, FAIL, - "can't allocate space for hyperslab selection 'count' values"); - if (NULL == (block = (hsize_t *)RV_malloc((size_t)ndims * sizeof(*block)))) - FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_CANTALLOC, FAIL, - "can't allocate space for hyperslab selection 'block' values"); - - if (nblocks = H5Sget_select_hyper_nblocks(space_id) < 0) - FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_CANTGET, FAIL, "can't get number of hyperslab blocks"); - - if (H5Sget_regular_hyperslab(space_id, start, stride, count, block) < 0) - FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_CANTGET, FAIL, "can't get regular hyperslab selection"); - - /* For contiguous, the stride should be 1. */ - for (i = 0; i < ndims; i++) { - if (stride[i] > 1) + switch (H5Sget_select_type(space_id)) { + case H5S_SEL_HYPERSLABS: { + if ((regular = H5Sis_regular_hyperslab(space_id)) < 0) + FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_CANTGET, FAIL, + "can't determine if the hyperslab is regular"); + if (!regular) FUNC_GOTO_DONE(FALSE); - } - if (nblocks > 1) { - /* Multiple blocks: count should be 1 except for the last dimension (fastest) */ - for (i = 0; i < ndims - 1; i++) { - if (count[i] > 1) + if (NULL == (dims = (hsize_t *)RV_malloc((size_t)ndims * sizeof(*dims)))) + FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_CANTALLOC, FAIL, + "can't allocate space for dimension 'dims' values"); + + if (H5Sget_simple_extent_dims(space_id, dims, NULL) < 0) + FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_CANTGET, FAIL, "can't get dataspace dimension size"); + + if (NULL == (start = (hsize_t *)RV_malloc((size_t)ndims * sizeof(*start)))) + FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_CANTALLOC, FAIL, + "can't allocate space for hyperslab selection 'start' values"); + if (NULL == (stride = (hsize_t *)RV_malloc((size_t)ndims * sizeof(*stride)))) + FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_CANTALLOC, FAIL, + "can't allocate space for hyperslab selection 'stride' values"); + if (NULL == (count = (hsize_t *)RV_malloc((size_t)ndims * sizeof(*count)))) + FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_CANTALLOC, FAIL, + "can't allocate space for hyperslab selection 'count' values"); + if (NULL == (block = (hsize_t *)RV_malloc((size_t)ndims * sizeof(*block)))) + FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_CANTALLOC, FAIL, + "can't allocate space for hyperslab selection 'block' values"); + + if (nblocks = H5Sget_select_hyper_nblocks(space_id) < 0) + FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_CANTGET, FAIL, "can't get number of hyperslab blocks"); + + if (H5Sget_regular_hyperslab(space_id, start, stride, count, block) < 0) + FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_CANTGET, FAIL, "can't get regular hyperslab selection"); + + /* For contiguous, the stride should be 1. */ + for (i = 0; i < ndims; i++) { + if (stride[i] > 1) FUNC_GOTO_DONE(FALSE); } - } - /* For contiguous, all faster running dimensions than the current dimension should be selected - * completely */ - whole = (start[ndims - 1] == 0) && (count[ndims - 1] * block[ndims - 1] == dims[ndims - 1]); - for (i = ndims - 2; i >= 0; i--) { - if ((dims[i] > 1) && (count[i] * block[i] > 1) && !whole) - FUNC_GOTO_DONE(FALSE); + if (nblocks > 1) { + /* Multiple blocks: count should be 1 except for the last dimension (fastest) */ + for (i = 0; i < ndims - 1; i++) { + if (count[i] > 1) + FUNC_GOTO_DONE(FALSE); + } + } - whole = whole && (start[i] == 0) && (count[i] * block[i] == dims[i]); - } - } /* end if*/ - else if (H5S_SEL_POINTS == H5Sget_select_type(space_id)) { - /* Assumption: any point selection is non-contiguous in memory */ - FUNC_GOTO_DONE(FALSE); - } /* end else if */ + /* For contiguous, all faster running dimensions than the current dimension should be selected + * completely */ + whole = (start[ndims - 1] == 0) && (count[ndims - 1] * block[ndims - 1] == dims[ndims - 1]); + for (i = ndims - 2; i >= 0; i--) { + if ((dims[i] > 1) && (count[i] * block[i] > 1) && !whole) + FUNC_GOTO_DONE(FALSE); + + whole = whole && (start[i] == 0) && (count[i] * block[i] == dims[i]); + } + break; + } /* H5S_SEL_HYPERSLABS */ + + case H5S_SEL_POINTS: + /* Assumption: any point selection is non-contiguous in memory */ + FUNC_GOTO_DONE(FALSE); + break; + + case H5S_SEL_ALL: + case H5S_SEL_ERROR: + case H5S_SEL_N: + case H5S_SEL_NONE: + default: + FUNC_GOTO_DONE(TRUE); + } /* end switch */ done: if (block) @@ -4287,40 +4306,58 @@ RV_convert_start_to_offset(hid_t space_id) { hsize_t *dims = NULL; hsize_t *start = NULL; + hsize_t *end = NULL; hssize_t ret_value = 0; int ndims, i; - if (H5S_SEL_HYPERSLABS == H5Sget_select_type(space_id)) { - if ((ndims = H5Sget_simple_extent_ndims(space_id)) < 0) - FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_CANTGET, -1, "can't retrieve dataspace dimensionality"); + switch (H5Sget_select_type(space_id)) { + case H5S_SEL_HYPERSLABS: { + if ((ndims = H5Sget_simple_extent_ndims(space_id)) < 0) + FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_CANTGET, -1, "can't retrieve dataspace dimensionality"); - if (NULL == (dims = (hsize_t *)RV_malloc((size_t)ndims * sizeof(*dims)))) - FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_CANTALLOC, -1, - "can't allocate space for dimension 'dims' values"); + if (NULL == (dims = (hsize_t *)RV_malloc((size_t)ndims * sizeof(*dims)))) + FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_CANTALLOC, -1, + "can't allocate space for dimension 'dims' values"); - if (H5Sget_simple_extent_dims(space_id, dims, NULL) < 0) - FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_CANTGET, -1, "can't get dataspace dimension size"); + if (H5Sget_simple_extent_dims(space_id, dims, NULL) < 0) + FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_CANTGET, -1, "can't get dataspace dimension size"); - if (NULL == (start = (hsize_t *)RV_malloc((size_t)ndims * sizeof(*start)))) - FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_CANTALLOC, -1, - "can't allocate space for hyperslab selection 'start' values"); + if (NULL == (start = (hsize_t *)RV_malloc((size_t)ndims * sizeof(*start)))) + FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_CANTALLOC, -1, + "can't allocate space for hyperslab selection 'start' values"); + if (NULL == (end = (hsize_t *)RV_malloc((size_t)ndims * sizeof(*end)))) + FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_CANTALLOC, -1, + "can't allocate space for hyperslab selection 'end' values"); - if (H5Sget_regular_hyperslab(space_id, start, NULL, NULL, NULL) < 0) - FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_CANTGET, -1, "can't get regular hyperslab selection"); + if (H5Sget_select_bounds(space_id, start, end) < 0) + FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_CANTGET, -1, + "can't get bounding box of hyperslab selection"); - ret_value = start[0]; - for (i = 1; i < ndims; i++) { - ret_value = ret_value * dims[i] + start[i]; - } - } /* end if */ - else if (H5S_SEL_POINTS == H5Sget_select_type(space_id)) { - FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_BADVALUE, -1, - "for point selection, computing the offset is not supported"); - } /* end else if */ + ret_value = start[0]; + for (i = 1; i < ndims; i++) { + ret_value = ret_value * dims[i] + start[i]; + } + break; + } /* H5S_SEL_HYPERSLABS */ + + case H5S_SEL_POINTS: + FUNC_GOTO_ERROR(H5E_DATASPACE, H5E_BADVALUE, -1, + "for point selection, computing the offset is not supported"); + break; + + case H5S_SEL_ALL: + case H5S_SEL_ERROR: + case H5S_SEL_N: + case H5S_SEL_NONE: + default: + ret_value = 0; + } /* end switch */ done: if (dims) RV_free(dims); + if (end) + RV_free(end); if (start) RV_free(start);