Fix support of bool (#654)

BlueBrain · Jan 24, 2023 · 8a93fdf · 8a93fdf
1 parent 4ad3555
commit 8a93fdf
Show file tree

Hide file tree

Showing 8 changed files with 337 additions and 30 deletions.
diff --git a/include/highfive/H5DataType.hpp b/include/highfive/H5DataType.hpp
@@ -403,10 +403,10 @@ class FixedLenStringArray {
 /// }
 /// HIGHFIVE_REGISTER_TYPE(FooBar, create_enum_foobar)
 /// \endcode
-#define HIGHFIVE_REGISTER_TYPE(type, function)             \
-    template <>                                            \
-    HighFive::DataType HighFive::create_datatype<type>() { \
-        return function();                                 \
+#define HIGHFIVE_REGISTER_TYPE(type, function)                    \
+    template <>                                                   \
+    inline HighFive::DataType HighFive::create_datatype<type>() { \
+        return function();                                        \
     }
 
 #include "bits/H5DataType_misc.hpp"
diff --git a/include/highfive/bits/H5Converter_misc.hpp b/include/highfive/bits/H5Converter_misc.hpp
@@ -81,8 +81,6 @@ struct type_helper {
     static constexpr size_t recursive_ndim = ndim;
     static constexpr bool is_trivially_copyable = std::is_trivially_copyable<type>::value;
 
-    static_assert(!std::is_same<type, bool>::value, "Booleans are not supported yet.");
-
     static std::vector<size_t> getDimensions(const type& /* val */) {
         return {};
     }
@@ -123,6 +121,36 @@ struct type_helper {
 template <typename T>
 struct inspector: type_helper<T> {};
 
+enum Boolean : int8_t {
+    FALSE = 0,
+    TRUE = 1,
+};
+template <>
+struct inspector<bool>: type_helper<bool> {
+    using base_type = Boolean;
+    using hdf5_type = int8_t;
+
+    static constexpr bool is_trivially_copyable = false;
+
+    static hdf5_type* data(type& /* val */) {
+        throw DataSpaceException("A boolean cannot be read directly.");
+    }
+
+    static const hdf5_type* data(const type& /* val */) {
+        throw DataSpaceException("A boolean cannot be written directly.");
+    }
+
+    static void unserialize(const hdf5_type* vec,
+                            const std::vector<size_t>& /* dims */,
+                            type& val) {
+        val = vec[0] != 0 ? true : false;
+    }
+
+    static void serialize(const type& val, hdf5_type* m) {
+        *m = val ? 1 : 0;
+    }
+};
+
 template <>
 struct inspector<std::string>: type_helper<std::string> {
     using hdf5_type = const char*;
@@ -132,7 +160,7 @@ struct inspector<std::string>: type_helper<std::string> {
     }
 
     static const hdf5_type* data(const type& /* val */) {
-        throw DataSpaceException("A std::string cannot be write directly.");
+        throw DataSpaceException("A std::string cannot be written directly.");
     }
 
     static void serialize(const type& val, hdf5_type* m) {
@@ -157,7 +185,7 @@ struct inspector<Reference>: type_helper<Reference> {
     }
 
     static const hdf5_type* data(const type& /* val */) {
-        throw DataSpaceException("A Reference cannot be write directly.");
+        throw DataSpaceException("A Reference cannot be written directly.");
     }
 
     static void serialize(const type& val, hdf5_type* m) {
@@ -237,7 +265,8 @@ struct inspector<std::vector<T>> {
 
     static constexpr size_t ndim = 1;
     static constexpr size_t recursive_ndim = ndim + inspector<value_type>::recursive_ndim;
-    static constexpr bool is_trivially_copyable = std::is_trivially_copyable<value_type>::value;
+    static constexpr bool is_trivially_copyable = std::is_trivially_copyable<value_type>::value &&
+                                                  inspector<value_type>::is_trivially_copyable;
 
     static std::vector<size_t> getDimensions(const type& val) {
         std::vector<size_t> sizes{val.size()};
@@ -259,7 +288,7 @@ struct inspector<std::vector<T>> {
     static void prepare(type& val, const std::vector<size_t>& dims) {
         val.resize(dims[0]);
         std::vector<size_t> next_dims(dims.begin() + 1, dims.end());
-        for (auto& e: val) {
+        for (auto&& e: val) {
             inspector<value_type>::prepare(e, next_dims);
         }
     }
@@ -274,7 +303,7 @@ struct inspector<std::vector<T>> {
 
     static void serialize(const type& val, hdf5_type* m) {
         size_t subsize = inspector<value_type>::getSizeVal(val[0]);
-        for (auto& e: val) {
+        for (auto&& e: val) {
             inspector<value_type>::serialize(e, m);
             m += subsize;
         }
@@ -291,17 +320,74 @@ struct inspector<std::vector<T>> {
     }
 };
 
+template <>
+struct inspector<std::vector<bool>> {
+    using type = std::vector<bool>;
+    using value_type = bool;
+    using base_type = Boolean;
+    using hdf5_type = uint8_t;
+
+    static constexpr size_t ndim = 1;
+    static constexpr size_t recursive_ndim = ndim;
+    static constexpr bool is_trivially_copyable = false;
+
+    static std::vector<size_t> getDimensions(const type& val) {
+        std::vector<size_t> sizes{val.size()};
+        return sizes;
+    }
+
+    static size_t getSizeVal(const type& val) {
+        return val.size();
+    }
+
+    static size_t getSize(const std::vector<size_t>& dims) {
+        if (dims.size() > 1) {
+            throw DataSpaceException("std::vector<bool> is only 1 dimension.");
+        }
+        return dims[0];
+    }
+
+    static void prepare(type& val, const std::vector<size_t>& dims) {
+        if (dims.size() > 1) {
+            throw DataSpaceException("std::vector<bool> is only 1 dimension.");
+        }
+        val.resize(dims[0]);
+    }
+
+    static hdf5_type* data(type& /* val */) {
+        throw DataSpaceException("A std::vector<bool> cannot be read directly.");
+    }
+
+    static const hdf5_type* data(const type& /* val */) {
+        throw DataSpaceException("A std::vector<bool> cannot be written directly.");
+    }
+
+    static void serialize(const type& val, hdf5_type* m) {
+        for (size_t i = 0; i < val.size(); ++i) {
+            m[i] = val[i] ? 1 : 0;
+        }
+    }
+
+    static void unserialize(const hdf5_type* vec_align,
+                            const std::vector<size_t>& dims,
+                            type& val) {
+        for (size_t i = 0; i < dims[0]; ++i) {
+            val[i] = vec_align[i] != 0 ? true : false;
+        }
+    }
+};
 
 template <typename T, size_t N>
 struct inspector<std::array<T, N>> {
     using type = std::array<T, N>;
-    using value_type = T;
+    using value_type = unqualified_t<T>;
     using base_type = typename inspector<value_type>::base_type;
     using hdf5_type = typename inspector<value_type>::hdf5_type;
 
     static constexpr size_t ndim = 1;
     static constexpr size_t recursive_ndim = ndim + inspector<value_type>::recursive_ndim;
-    static constexpr bool is_trivially_copyable = std::is_trivially_copyable<value_type>::value;
+    static constexpr bool is_trivially_copyable = std::is_trivially_copyable<value_type>::value &&
+                                                  inspector<value_type>::is_trivially_copyable;
 
     static std::vector<size_t> getDimensions(const type& val) {
         std::vector<size_t> sizes{N};
@@ -371,7 +457,8 @@ struct inspector<T*> {
 
     static constexpr size_t ndim = 1;
     static constexpr size_t recursive_ndim = ndim + inspector<value_type>::recursive_ndim;
-    static constexpr bool is_trivially_copyable = std::is_trivially_copyable<value_type>::value;
+    static constexpr bool is_trivially_copyable = std::is_trivially_copyable<value_type>::value &&
+                                                  inspector<value_type>::is_trivially_copyable;
 
     static size_t getSizeVal(const type& /* val */) {
         throw DataSpaceException("Not possible to have size of a T*");
@@ -402,7 +489,8 @@ struct inspector<T[N]> {
 
     static constexpr size_t ndim = 1;
     static constexpr size_t recursive_ndim = ndim + inspector<value_type>::recursive_ndim;
-    static constexpr bool is_trivially_copyable = std::is_trivially_copyable<value_type>::value;
+    static constexpr bool is_trivially_copyable = std::is_trivially_copyable<value_type>::value &&
+                                                  inspector<value_type>::is_trivially_copyable;
 
     static size_t getSizeVal(const type& val) {
         return compute_total_size(getDimensions(val));
@@ -441,7 +529,8 @@ struct inspector<Eigen::Matrix<T, M, N>> {
 
     static constexpr size_t ndim = 2;
     static constexpr size_t recursive_ndim = ndim + inspector<value_type>::recursive_ndim;
-    static constexpr bool is_trivially_copyable = std::is_trivially_copyable<value_type>::value;
+    static constexpr bool is_trivially_copyable = std::is_trivially_copyable<value_type>::value &&
+                                                  inspector<value_type>::is_trivially_copyable;
 
     static std::vector<size_t> getDimensions(const type& val) {
         std::vector<size_t> sizes{static_cast<size_t>(val.rows()), static_cast<size_t>(val.cols())};
@@ -502,7 +591,8 @@ struct inspector<boost::multi_array<T, Dims>> {
 
     static constexpr size_t ndim = Dims;
     static constexpr size_t recursive_ndim = ndim + inspector<value_type>::recursive_ndim;
-    static constexpr bool is_trivially_copyable = std::is_trivially_copyable<value_type>::value;
+    static constexpr bool is_trivially_copyable = std::is_trivially_copyable<value_type>::value &&
+                                                  inspector<value_type>::is_trivially_copyable;
 
     static std::vector<size_t> getDimensions(const type& val) {
         std::vector<size_t> sizes;
@@ -580,7 +670,8 @@ struct inspector<boost::numeric::ublas::matrix<T>> {
 
     static constexpr size_t ndim = 2;
     static constexpr size_t recursive_ndim = ndim + inspector<value_type>::recursive_ndim;
-    static constexpr bool is_trivially_copyable = std::is_trivially_copyable<value_type>::value;
+    static constexpr bool is_trivially_copyable = std::is_trivially_copyable<value_type>::value &&
+                                                  inspector<value_type>::is_trivially_copyable;
 
     static std::vector<size_t> getDimensions(const type& val) {
         std::vector<size_t> sizes{val.size1(), val.size2()};

diff --git a/include/highfive/bits/H5DataType_misc.hpp b/include/highfive/bits/H5DataType_misc.hpp
@@ -160,12 +160,6 @@ inline AtomicType<long double>::AtomicType() {
     _hid = H5Tcopy(H5T_NATIVE_LDOUBLE);
 }
 
-// boolean mapping
-template <>
-inline AtomicType<bool>::AtomicType() {
-    _hid = H5Tcopy(H5T_NATIVE_HBOOL);
-}
-
 // std string
 template <>
 inline AtomicType<std::string>::AtomicType() {
@@ -200,6 +194,11 @@ class AtomicType<std::complex<T>>: public DataType {
     }
 };
 
+// For boolean we act as h5py
+inline EnumType<details::Boolean> create_enum_boolean() {
+    return {{"FALSE", details::Boolean::FALSE}, {"TRUE", details::Boolean::TRUE}};
+}
+
 // Other cases not supported. Fail early with a user message
 template <typename T>
 AtomicType<T>::AtomicType() {
@@ -493,3 +492,4 @@ inline DataType create_and_check_datatype() {
 }
 
 }  // namespace HighFive
+HIGHFIVE_REGISTER_TYPE(HighFive::details::Boolean, HighFive::create_enum_boolean)
diff --git a/include/highfive/bits/H5Node_traits.hpp b/include/highfive/bits/H5Node_traits.hpp
@@ -12,6 +12,7 @@
 
 #include "../H5PropertyList.hpp"
 #include "H5_definitions.hpp"
+#include "H5Converter_misc.hpp"
 
 namespace HighFive {
 
@@ -47,7 +48,20 @@ class NodeTraits {
     /// \param accessProps A property list with data set access properties
     /// \param parents Create intermediate groups if needed. Default: true.
     /// \return DataSet Object
-    template <typename Type>
+    template <typename T,
+              typename std::enable_if<
+                  std::is_same<typename details::inspector<T>::base_type, details::Boolean>::value,
+                  int>::type* = nullptr>
+    DataSet createDataSet(const std::string& dataset_name,
+                          const DataSpace& space,
+                          const DataSetCreateProps& createProps = DataSetCreateProps::Default(),
+                          const DataSetAccessProps& accessProps = DataSetAccessProps::Default(),
+                          bool parents = true);
+
+    template <typename T,
+              typename std::enable_if<
+                  !std::is_same<typename details::inspector<T>::base_type, details::Boolean>::value,
+                  int>::type* = nullptr>
     DataSet createDataSet(const std::string& dataset_name,
                           const DataSpace& space,
                           const DataSetCreateProps& createProps = DataSetCreateProps::Default(),

diff --git a/include/highfive/bits/H5Node_traits_misc.hpp b/include/highfive/bits/H5Node_traits_misc.hpp
@@ -54,14 +54,35 @@ inline DataSet NodeTraits<Derivate>::createDataSet(const std::string& dataset_na
 }
 
 template <typename Derivate>
-template <typename Type>
+template <typename T,
+          typename std::enable_if<
+              std::is_same<typename details::inspector<T>::base_type, details::Boolean>::value,
+              int>::type*>
+inline DataSet NodeTraits<Derivate>::createDataSet(const std::string& dataset_name,
+                                                   const DataSpace& space,
+                                                   const DataSetCreateProps& createProps,
+                                                   const DataSetAccessProps& accessProps,
+                                                   bool parents) {
+    return createDataSet(dataset_name,
+                         space,
+                         create_and_check_datatype<typename details::inspector<T>::base_type>(),
+                         createProps,
+                         accessProps,
+                         parents);
+}
+
+template <typename Derivate>
+template <typename T,
+          typename std::enable_if<
+              !std::is_same<typename details::inspector<T>::base_type, details::Boolean>::value,
+              int>::type*>
 inline DataSet NodeTraits<Derivate>::createDataSet(const std::string& dataset_name,
                                                    const DataSpace& space,
                                                    const DataSetCreateProps& createProps,
                                                    const DataSetAccessProps& accessProps,
                                                    bool parents) {
     return createDataSet(
-        dataset_name, space, create_and_check_datatype<Type>(), createProps, accessProps, parents);
+        dataset_name, space, create_and_check_datatype<T>(), createProps, accessProps, parents);
 }
 
 template <typename Derivate>

diff --git a/tests/test_project_integration.sh b/tests/test_project_integration.sh
@@ -16,7 +16,7 @@ test_install() {
     mkdir -p "${builddir}"
     pushd "${builddir}"
     cmake "${TESTDIR}/${project}" "$@"
-    make VERBOSE=1
+    cmake --build . --verbose
     ctest
     popd
     rm "${TESTDIR}/${project}/deps/HighFive"
@@ -29,7 +29,7 @@ cmake "${ROOT}" \
     -DHIGHFIVE_EXAMPLES=OFF \
     -DHIGHFIVE_UNIT_TESTS=OFF \
     -DCMAKE_INSTALL_PREFIX="${PWD}/install"
-make install
+cmake --build . --target install
 popd
 
 for project in test_project test_dependent_library; do

diff --git a/tests/unit/CMakeLists.txt b/tests/unit/CMakeLists.txt
@@ -6,7 +6,7 @@ if(MSVC)
 endif()
 
 ## Base tests
-foreach(test_name tests_high_five_base tests_high_five_multi_dims tests_high_five_easy)
+foreach(test_name tests_high_five_base tests_high_five_multi_dims tests_high_five_easy test_all_types)
   add_executable(${test_name} "${test_name}.cpp")
   target_link_libraries(${test_name} HighFive Catch2::Catch2WithMain)
   catch_discover_tests(${test_name})