Merge pull request #141 from grimbough/unlimited

Unlimited
grimbough · Mar 5, 2024 · 66bae61 · 66bae61
2 parents 77322cb + e22b821
commit 66bae61
Show file tree

Hide file tree

Showing 8 changed files with 138 additions and 45 deletions.
diff --git a/DESCRIPTION b/DESCRIPTION
@@ -1,7 +1,7 @@
 Package: rhdf5
 Type: Package
 Title: R Interface to HDF5
-Version: 2.47.4
+Version: 2.47.5
 Authors@R: c(person("Bernd", "Fischer", role = c("aut")), 
         person("Mike", "Smith", 
             role=c("aut", "cre"), 
@@ -33,4 +33,4 @@ SystemRequirements: GNU make
 biocViews: Infrastructure, DataImport
 Encoding: UTF-8
 Roxygen: list(markdown = TRUE)
-RoxygenNote: 7.2.3
+RoxygenNote: 7.3.1
diff --git a/R/H5S.R b/R/H5S.R
@@ -29,9 +29,9 @@ H5Screate <- function( type = h5default("H5S"), native = FALSE ) {
 
 #' Create a simple dataspace
 #'
-#' @param dims An integer vector defining the initial dimensions of the dataspace.
+#' @param dims A numeric vector defining the initial dimensions of the dataspace.
 #' The length of `dims` determines the rank of the dataspace.
-#' @param maxdims An integer vector with the same length length as `dims`.  Specifies the 
+#' @param maxdims A numeric vector with the same length length as `dims`.  Specifies the
 #' upper limit on the size of the dataspace dimensions.  Only needs to be specified
 #' if this is different from the values given to `dims`.
 #' @param native An object of class `logical`. If `TRUE`, array-like
@@ -47,11 +47,12 @@ H5Screate <- function( type = h5default("H5S"), native = FALSE ) {
 #'
 #' @export
 H5Screate_simple <- function( dims, maxdims, native = FALSE ) {
+  dims <- as.numeric(dims)
   if (missing(maxdims)) {
-    maxdims = dims
+    maxdims <- dims
+  } else {
+    maxdims <- as.numeric(maxdims)
   }
-  dims <- as.integer(dims)
-  maxdims <- as.integer(maxdims)
   if (!native) {
     dims <- rev(dims)
     maxdims <- rev(maxdims)
@@ -139,21 +140,26 @@ H5Sget_simple_extent_dims <- function( h5space ) {
 #' 
 #' @param h5space [H5IdComponent-class] object representing a dataspace.
 #' @param dims Dimension of the dataspace. This argument is similar to the dim
-#' attribute of an array. When viewing the HDF5 dataset with an C-program 
-#' (e.g. HDFView), the dimensions appear in inverted order, because the 
-#' fastest changing dimension in R is the first one, and in C its the last 
-#' one.
+#' attribute of an array. 
 #' @param maxdims Maximum extension of the dimension of the dataset in the 
 #' file. If not provided, it is set to `dims`.
 #' 
+#' When viewing the HDF5 dataset with other software 
+#' (e.g. HDFView), the dimensions appear in inverted order, because the 
+#' fastest changing dimension in R is the first one, and in C it's the last 
+#' one.
+#' 
 #' @export
 H5Sset_extent_simple <- function( h5space, dims, maxdims) {
   h5checktype(h5space, "dataspace")
+
+  dims <- as.numeric(dims)
   if (missing(maxdims)) {
-    maxdims = dims
+    maxdims <- dims
+  } else {
+    maxdims <- as.numeric(maxdims)
   }
-  dims <- as.integer(dims)
-  maxdims <- as.integer(maxdims)
+
   if (!h5space@native){
       dims <- rev(dims)
       maxdims <- rev(maxdims)

diff --git a/inst/NEWS.Rd b/inst/NEWS.Rd
@@ -8,6 +8,12 @@
       \item R complex types can now be written to HDF5.  These will be stored as
       a compound datatype with two elements (r & i) representing the real and
       imaginary parts.
+      \item Functions \code{H5Screate_simple} and \code{H5Sset_extent_simple} 
+      now accept numeric values to the \code{dim} and \code{maxdim} arguments,
+      allowing the creation of HDF5 dataspaces larger than R's maximum
+      integer value.
+      (Thanks to @hpages for reporting this and providing a patch
+      https://github.com/grimbough/rhdf5/pull/140).
     }
   }
 

diff --git a/man/H5Screate_simple.Rd b/man/H5Screate_simple.Rd
diff --git a/man/H5Sset_extent_simple.Rd b/man/H5Sset_extent_simple.Rd
diff --git a/src/H5D.c b/src/H5D.c
@@ -682,9 +682,8 @@ int is_complex(hid_t dtype_id) {
     char *field1 = H5Tget_member_name(dtype_id, 0);
     char *field2 = H5Tget_member_name(dtype_id, 1);
 
-    if((strcmp(field1, "r") == 0) && (strcmp(field2, "i") == 0)) {
+    if((strcmp(field1, "r") == 0) && (strcmp(field2, "i") == 0))
       res = 1;
-    }
 
     free(field1);
     free(field2);
@@ -702,6 +701,7 @@ SEXP H5Dread_helper_COMPLEX(hid_t dataset_id, hid_t file_space_id, hid_t mem_spa
 
   herr_t herr = H5Dread(dataset_id, dtype_id, mem_space_id, file_space_id, H5P_DEFAULT, buf );
   if(herr < 0) {
+    UNPROTECT(1);
     error("Unable to read dataset");
   }
 
@@ -718,11 +718,6 @@ SEXP H5Dread_helper_COMPOUND(hid_t dataset_id, hid_t file_space_id, hid_t mem_sp
                              int bit64conversion, int native ) {
 
     SEXP Rval;
-
-    if(is_complex(dtype_id)) {
-        Rval = H5Dread_helper_COMPLEX(dataset_id, file_space_id, mem_space_id, n, Rdim, dtype_id, native);
-        return(Rval);
-    }
 
     if ((LENGTH(Rdim) > 1) && compoundAsDataFrame) {
         compoundAsDataFrame = 0;
@@ -784,6 +779,23 @@ SEXP H5Dread_helper_COMPOUND(hid_t dataset_id, hid_t file_space_id, hid_t mem_sp
     return(Rval);
 }
 
+SEXP H5Dread_helper_COMPOUND_OR_COMPLEX(
+    hid_t dataset_id, hid_t file_space_id, hid_t mem_space_id, hsize_t n, SEXP Rdim, SEXP _buf, 
+    hid_t dtype_id, hid_t cpdType, int cpdNField, char ** cpdField, int compoundAsDataFrame,
+    int bit64conversion, int native ) {
+
+  SEXP Rval;
+
+  if(is_complex(dtype_id)) {
+    Rval = H5Dread_helper_COMPLEX(dataset_id, file_space_id, mem_space_id, n, Rdim, dtype_id, native);
+  } else {
+    Rval = H5Dread_helper_COMPOUND(dataset_id, file_space_id, mem_space_id, n, Rdim, _buf, 
+                                   dtype_id, cpdType, cpdNField, cpdField, compoundAsDataFrame,
+                                   bit64conversion, native);
+  }
+
+  return(Rval);
+}
 
 //SEXP H5Dread_helper_REFERENCE(hid_t attr_id, hsize_t n, SEXP Rdim, SEXP _buf, hid_t dtype_id) {
 SEXP H5Dread_helper_REFERENCE(hid_t dataset_id, hid_t file_space_id, hid_t mem_space_id, hsize_t n, SEXP Rdim, SEXP _buf,
@@ -848,7 +860,7 @@ SEXP H5Dread_helper(hid_t dataset_id, hid_t file_space_id, hid_t mem_space_id, h
                                      dtype_id, cpdType, cpdNField, cpdField, compoundAsDataFrame, native );
     } break;
     case H5T_COMPOUND: {
-        Rval = H5Dread_helper_COMPOUND(dataset_id, file_space_id, mem_space_id, n, Rdim, _buf, 
+        Rval = H5Dread_helper_COMPOUND_OR_COMPLEX(dataset_id, file_space_id, mem_space_id, n, Rdim, _buf, 
                                        dtype_id, cpdType, cpdNField, cpdField, compoundAsDataFrame,
                                        bit64conversion, native );
     } break;

diff --git a/src/H5S.c b/src/H5S.c
@@ -45,7 +45,7 @@ SEXP _H5Screate_simple( SEXP _dims, SEXP _maxdims ) {
     int rank = length(_dims);
     hsize_t dims[rank];
     for (int i=0; i<rank; i++) {
-        dims[i] = (hsize_t) INTEGER(_dims)[i];
+        dims[i] = (hsize_t) REAL(_dims)[i];
     }
     if (length(_maxdims) == 0) {
         hid = H5Screate_simple( rank, dims, NULL);
@@ -56,12 +56,11 @@ SEXP _H5Screate_simple( SEXP _dims, SEXP _maxdims ) {
             hid = -1;
         } else {
             hsize_t maxdims[rank];
+            double maxdim;
             for (int i=0; i<rank; i++) {
-                maxdims[i] = (hsize_t) INTEGER(_maxdims)[i];
                 // We explicitly use -1 to represent unlimited dimensions
-                if(maxdims[i] == -1) {
-                  maxdims[i] = H5S_UNLIMITED;
-                }
+                maxdim = REAL(_maxdims)[i];
+                maxdims[i] = maxdim < 0 ? H5S_UNLIMITED : (hsize_t) maxdim;
             }
             hid = H5Screate_simple( rank, dims, maxdims);
             addHandle(hid);
@@ -108,16 +107,16 @@ SEXP _H5Sget_simple_extent_dims( SEXP _space_id ) {
     } else {
         for (int i=0; i < rank; i++) {
             size_is_numeric += size[i] > R_LEN_T_MAX;
-            maxsize_is_numeric += maxsize[i] > R_LEN_T_MAX;
+            maxsize_is_numeric += (maxsize[i] > R_LEN_T_MAX) & (maxsize[i] != H5S_UNLIMITED);
         }
         Rsize = PROTECT(allocVector(REALSXP, rank));
         Rmaxsize = PROTECT(allocVector(REALSXP, rank));
         for (int i=0; i < rank; i++) {
-            REAL(Rsize)[i] = size[i];
-            REAL(Rmaxsize)[i] = maxsize[i];
+            REAL(Rsize)[i] = (double) size[i];
+            REAL(Rmaxsize)[i] = (maxsize[i] == H5S_UNLIMITED) ? -1 : (double) maxsize[i];
         }
-        SET_VECTOR_ELT(Rval,1,Rsize);
-        SET_VECTOR_ELT(Rval,2,Rmaxsize);
+        SET_VECTOR_ELT(Rval,1, Rsize);
+        SET_VECTOR_ELT(Rval,2, Rmaxsize);
         UNPROTECT(2);
     }
 
@@ -143,7 +142,7 @@ SEXP _H5Sset_extent_simple( SEXP _space_id, SEXP _current_size, SEXP _maximum_si
     int rank = length(_current_size);
     hsize_t current_size[rank];
     for (int i=0; i<rank; i++) {
-        current_size[i] = (hsize_t) INTEGER(_current_size)[i];
+        current_size[i] = (hsize_t) REAL(_current_size)[i];
     }
     if (length(_maximum_size) == 0) {
         herr = H5Sset_extent_simple( space_id, rank, current_size, NULL);
@@ -153,12 +152,11 @@ SEXP _H5Sset_extent_simple( SEXP _space_id, SEXP _current_size, SEXP _maximum_si
             herr = -1;
         } else {
             hsize_t maximum_size[rank];
+            double maxdim;
             for (int i=0; i<rank; i++) {
-                maximum_size[i] = (hsize_t) INTEGER(_maximum_size)[i];
-                // We explicitly use -1 to represent unlimited dimensions
-                if(maximum_size[i] == -1) {
-                  maximum_size[i] = H5S_UNLIMITED;
-                }
+              // We explicitly use -1 to represent unlimited dimensions
+              maxdim = REAL(_maximum_size)[i];
+              maximum_size[i] = maxdim < 0 ? H5S_UNLIMITED : (hsize_t) maxdim;
             }
             herr = H5Sset_extent_simple( space_id, rank, current_size, maximum_size);
         }

diff --git a/tests/testthat/test_H5S.R b/tests/testthat/test_H5S.R
@@ -52,6 +52,11 @@ test_that("We can create a simple dataspace", {
     expect_equal(dspace_dims$size, dspace_dims$maxsize)
     expect_equal(dspace_dims$rank, 2)
 
+    ## if maxdims isn't supplied, it just matches dims
+    expect_silent(H5Sset_extent_simple(sid, dims = c(1,1,1)))
+    dspace_dims <- H5Sget_simple_extent_dims(sid)
+    expect_equal(dspace_dims$size, dspace_dims$maxsize)
+
     expect_silent(H5Sset_extent_simple(sid, dims = c(1,2,3), maxdims = c(10,20,30)))
     expect_is(dspace_dims <- H5Sget_simple_extent_dims(sid), "list")
     expect_false( identical(dspace_dims$size, dspace_dims$maxsize) )
@@ -60,6 +65,20 @@ test_that("We can create a simple dataspace", {
     expect_silent(H5Sclose(sid))
 })
 
+test_that("We can create a dataspace with unlimited or non 32-bit integer dims", {
+  expect_silent(sid <- H5Screate_simple(dims = c(10, 2), maxdims = c(10, 3e9)))
+  dspace_dims <- H5Sget_simple_extent_dims(sid)
+  expect_is(dspace_dims$maxsize, 'numeric')
+  expect_equal(dspace_dims$maxsize, c(10, 3e9))
+
+  expect_silent(H5Sset_extent_simple(sid, dims = c(1,2), maxdims = c(10, H5Sunlimited())))
+  dspace_dims <- H5Sget_simple_extent_dims(sid)
+  expect_is(dspace_dims$maxsize, 'integer')
+  expect_equal( dspace_dims$maxsize, c(10, -1) )
+
+  expect_silent(H5Sclose(sid))
+})
+
 ############################################################
 context("H5S Select Hyperslab")
 ############################################################
@@ -107,6 +126,56 @@ test_that("Selecting using an index", {
     expect_silent(H5Sclose(sid))
 })
 
+test_that("Other selection functions", {
+
+  dims <- c(10,20,30)
+  expect_silent(sid <- H5Screate_simple(dims = dims))
+
+  expect_silent( H5Sselect_all(sid) )
+  expect_identical( H5Sget_select_npoints(sid), prod(dims) )
+
+  expect_silent( H5Sselect_none(sid) )
+  expect_identical( H5Sget_select_npoints(sid), 0 )
+
+  expect_silent(H5Sclose(sid))
+})
+
+test_that("Combining selections", {
+
+  sid_1 <- H5Screate_simple(dims = 20)
+  sid_2 <- H5Screate_simple(dims = 10)
+
+  ## select a single block of 5 points in sid_1
+  ## this is equivalent to [11:16] in R syntax
+  H5Sselect_hyperslab(sid_1, start = 11, stride = 1,
+                      block = 5, count = 1)
+
+  ## select 2 blocks of 1 point from sid_2
+  ## equivalent to [c(3,5)] in R syntax
+  H5Sselect_hyperslab(sid_2, start = 3, stride = 2,
+                      block = 1, count = 2)
+
+  ## confirm we have select 5 and 2 points resepectively
+  expect_equal(H5Sget_select_npoints(sid_1), 5)
+  expect_equal(H5Sget_select_npoints(sid_2), 2)
+
+  ## combine the two dataset selections keeping points that
+  ## are in one or both of the selections
+  sid_3 <- H5Scombine_select(sid_1, "H5S_SELECT_OR", sid_2)
+
+  ## extent of the new dataset is the same as sid_1
+  sid_3
+  ## confirm the selection contains 7 points
+  expect_equal(H5Sget_select_npoints(sid_3), 7)
+
+  ## tidy up
+  H5Sclose(sid_1)
+  H5Sclose(sid_2)
+  H5Sclose(sid_3)
+
+
+})
+
 ############################################################
 context("H5S cleanup")
 ##########################################################