code review

src/Simplex_tree/concept/FiltrationValue.h

@@ -93,13 +93,13 @@ struct FiltrationValue {
    * an array of char, which was previously serialized by @ref Gudhi::serialize_value_to_char_buffer "".
    * Then, sets the value with it.
    * Overloads for native arithmetic types or other simple types are already implemented with
-   * @ref Gudhi::deserialize_value_to_char_buffer "".
+   * @ref Gudhi::deserialize_value_from_char_buffer "".
    * 
    * @param value The value where to deserialize based on its type.
    * @param start Start position where the value is serialized.
    * @return The new position in the array of char for the next deserialization.
    */
-  friend const char* deserialize_value_to_char_buffer(FiltrationValue& value, const char* start);
+  friend const char* deserialize_value_from_char_buffer(FiltrationValue& value, const char* start);
 
   /**
    * @brief Only necessary when serializing the simplex tree. Returns the serialization size of the given object.

src/Simplex_tree/include/gudhi/Simplex_tree.h

@@ -2764,21 +2764,22 @@ class Simplex_tree {
    */
   void deserialize(const char* buffer, const std::size_t buffer_size) {
     deserialize(buffer, buffer_size, [](Filtration_value& filtration, const char* ptr) {
-      return deserialize_value_to_char_buffer(filtration, ptr);
+      return deserialize_value_from_char_buffer(filtration, ptr);
     });
   }
 
   /** @private @brief Deserialize the array of char (flatten version of the tree) to initialize a Simplex tree.
    * It is the user's responsibility to provide an 'empty' Simplex_tree, there is no guarantee otherwise.
    * 
-   * @tparam F Method taking an @ref Filtration_value and a `const char*` as input and returning a
+   * @tparam F Method taking a reference to a @ref Filtration_value and a `const char*` as input and returning a
    * `const char*`.
    * @param[in] buffer A pointer on a buffer that contains a serialized Simplex_tree.
    * @param[in] buffer_size The size of the buffer.
-   * @param[in] deserialize_filtration_value To provide if the type of @ref Filtration_value is not trivially
-   * convertible from the filtration value type of the serialized simplex tree. Takes the filtration value to fill and
-   * a pointer to the current position in the buffer as arguments and returns the new position of the pointer after
-   * reading the filtration value and transforming it into a element of the host's filtration value type.
+   * @param[in] deserialize_filtration_value To provide if the type of @ref Filtration_value does not correspond to
+   * the filtration value type of the serialized simplex tree. The method should be able to read from a buffer
+   * (second argument) the serialized filtration value and turn it into an object of type @ref Filtration_value that is
+   * stored in the first argument of the method. It then returns the new position of the buffer pointer after the
+   * reading.
    * 
    * @exception std::invalid_argument In case the deserialization does not finish at the correct buffer_size.
    * @exception std::logic_error In debug mode, if the Simplex_tree is not 'empty'.
@@ -2793,7 +2794,7 @@ class Simplex_tree {
     const char* ptr = buffer;
     // Needs to read size before recursivity to manage new siblings for children
     Vertex_handle members_size;
-    ptr = deserialize_value_to_char_buffer(members_size, ptr);
+    ptr = deserialize_value_from_char_buffer(members_size, ptr);
     ptr = rec_deserialize(&root_, members_size, ptr, 0, deserialize_filtration_value);
     if (static_cast<std::size_t>(ptr - buffer) != buffer_size) {
       throw std::invalid_argument("Deserialization does not match end of buffer");
@@ -2815,9 +2816,9 @@ class Simplex_tree {
       Vertex_handle vertex;
       // Set explicitly to zero to avoid false positive error raising in debug mode when store_filtration == false
       // and to force array like Filtration_value value to be empty.
-      Filtration_value filtration = Gudhi::simplex_tree::empty_filtration_value;
+      Filtration_value filtration(Gudhi::simplex_tree::empty_filtration_value);
       for (Vertex_handle idx = 0; idx < members_size; idx++) {
-        ptr = deserialize_value_to_char_buffer(vertex, ptr);
+        ptr = deserialize_value_from_char_buffer(vertex, ptr);
         if constexpr (Options::store_filtration) {
           ptr = deserialize_filtration_value(filtration, ptr);
         }
@@ -2828,7 +2829,7 @@ class Simplex_tree {
       Vertex_handle child_size;
       for (auto sh = sib->members().begin(); sh != sib->members().end(); ++sh) {
         update_simplex_tree_after_node_insertion(sh);
-        ptr = deserialize_value_to_char_buffer(child_size, ptr);
+        ptr = deserialize_value_from_char_buffer(child_size, ptr);
         if (child_size > 0) {
           Siblings* child = new Siblings(sib, sh->first);
           sh->second.assign_children(child);

src/Simplex_tree/include/gudhi/Simplex_tree/filtration_value_utils.h

@@ -40,7 +40,7 @@ inline struct empty_filtration_value_t {
  * NaN values are not supported.
  */
 template <typename Arithmetic_filtration_value>
-bool unify_lifetimes(Arithmetic_filtration_value& f1, Arithmetic_filtration_value f2)
+bool unify_lifetimes(Arithmetic_filtration_value& f1, const Arithmetic_filtration_value& f2)
 {
   if (f2 < f1){
     f1 = f2;
@@ -57,7 +57,7 @@ bool unify_lifetimes(Arithmetic_filtration_value& f1, Arithmetic_filtration_valu
  * Because the filtration values are totally ordered then, the upper bound is always the maximum of the two values.
  */
 template <typename Arithmetic_filtration_value>
-bool intersect_lifetimes(Arithmetic_filtration_value& f1, Arithmetic_filtration_value f2)
+bool intersect_lifetimes(Arithmetic_filtration_value& f1, const Arithmetic_filtration_value& f2)
 {
   if constexpr (std::numeric_limits<Arithmetic_filtration_value>::has_quiet_NaN) {
     if (std::isnan(f1)) {
@@ -81,10 +81,13 @@ bool intersect_lifetimes(Arithmetic_filtration_value& f1, Arithmetic_filtration_
 }
 
 /**
+ * @private
  * @ingroup simplex_tree
  * @brief Serialize the given value and insert it at start position using
  * @ref Gudhi::simplex_tree::serialize_trivial "".
  * 
+ * @tparam Trivial_filtration_value Type which can trivially be serialized with byte to byte copy of the content
+ * of the holding variable. E.g., native arithmetic types.
  * @param[in] value The value to serialize.
  * @param[in] start Start position where the value is serialized.
  * @return The new position in the array of char for the next serialization.
@@ -97,21 +100,37 @@ char* serialize_value_to_char_buffer(Trivial_filtration_value value, char* start
 }
 
 /**
+ * @private
  * @ingroup simplex_tree
  * @brief Deserialize at the start position in an array of char and sets the value with it using
  * @ref Gudhi::simplex_tree::deserialize_trivial "".
  * 
+ * @tparam Trivial_filtration_value Type which can trivially be serialized with byte to byte copy of the content
+ * of the holding variable. E.g., native arithmetic types.
  * @param[in] value The value where to deserialize based on its type.
  * @param[in] start Start position where the value is serialized.
  * @return The new position in the array of char for the next deserialization.
  * 
  * @warning It is the user's responsibility to ensure that the pointer will not go out of bounds.
  */
 template <typename Trivial_filtration_value>
-const char* deserialize_value_to_char_buffer(Trivial_filtration_value& value, const char* start) {
+const char* deserialize_value_from_char_buffer(Trivial_filtration_value& value, const char* start) {
   return Gudhi::simplex_tree::deserialize_trivial(value, start);
 }
 
+/**
+ * @private
+ * @ingroup simplex_tree
+ * @brief Returns the size of the template type `Trivial_filtration_value`.
+ * 
+ * @tparam Trivial_filtration_value Type which can trivially be serialized with byte to byte copy of the content
+ * of the holding variable. E.g., native arithmetic types.
+ */
+template<typename Trivial_filtration_value>
+constexpr std::size_t get_serialization_size_of([[maybe_unused]] Trivial_filtration_value value) {
+  return sizeof(Trivial_filtration_value);
+}
+
 }  // namespace Gudhi
 
 #endif  // SIMPLEX_TREE_FILTRATION_VALUE_UTILS_H_

src/Simplex_tree/include/gudhi/Simplex_tree/serialization_utils.h

@@ -51,15 +51,6 @@ const char* deserialize_trivial(ArgumentType& value, const char* start) {
   return (start + arg_size);
 }
 
-/**
- * @ingroup simplex_tree
- * @brief Returns the size of the serialization of the given object.
- */
-template<class ArgumentType>
-constexpr std::size_t get_serialization_size_of([[maybe_unused]] ArgumentType value) {
-  return sizeof(ArgumentType);
-}
-
 }  // namespace simplex_tree
 
 }  // namespace Gudhi

src/Simplex_tree/test/simplex_tree_serialization_unit_test.cpp

@@ -162,40 +162,40 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(basic_simplex_tree_serialization, Stree, list_of_t
   // Reset position pointer at start
   const char* c_ptr = buffer;
   // 3 simplices ({0}, {1}, {2}) and its filtration values
-  c_ptr = deserialize_value_to_char_buffer(vertex, c_ptr);
+  c_ptr = deserialize_value_from_char_buffer(vertex, c_ptr);
   BOOST_CHECK(vertex == 3);
-  c_ptr = deserialize_value_to_char_buffer(vertex, c_ptr);
+  c_ptr = deserialize_value_from_char_buffer(vertex, c_ptr);
   BOOST_CHECK(vertex == 0);
   if (Stree::Options::store_filtration) {
-    c_ptr = deserialize_value_to_char_buffer(filtration, c_ptr);
+    c_ptr = deserialize_value_from_char_buffer(filtration, c_ptr);
     test_equality(filtration, st.filtration(st.find({0})));
   }
-  c_ptr = deserialize_value_to_char_buffer(vertex, c_ptr);
+  c_ptr = deserialize_value_from_char_buffer(vertex, c_ptr);
   BOOST_CHECK(vertex == 1);
   if (Stree::Options::store_filtration) {
-    c_ptr = deserialize_value_to_char_buffer(filtration, c_ptr);
+    c_ptr = deserialize_value_from_char_buffer(filtration, c_ptr);
     test_equality(filtration, st.filtration(st.find({1})));
   }
-  c_ptr = deserialize_value_to_char_buffer(vertex, c_ptr);
+  c_ptr = deserialize_value_from_char_buffer(vertex, c_ptr);
   BOOST_CHECK(vertex == 2);
   if (Stree::Options::store_filtration) {
-    c_ptr = deserialize_value_to_char_buffer(filtration, c_ptr);
+    c_ptr = deserialize_value_from_char_buffer(filtration, c_ptr);
     test_equality(filtration, st.filtration(st.find({2})));
   }
   // 1 simplex (2) from {0, 2} and its filtration values
-  c_ptr = deserialize_value_to_char_buffer(vertex, c_ptr);
+  c_ptr = deserialize_value_from_char_buffer(vertex, c_ptr);
   BOOST_CHECK(vertex == 1);
-  c_ptr = deserialize_value_to_char_buffer(vertex, c_ptr);
+  c_ptr = deserialize_value_from_char_buffer(vertex, c_ptr);
   BOOST_CHECK(vertex == 2);
   if (Stree::Options::store_filtration) {
-    c_ptr = deserialize_value_to_char_buffer(filtration, c_ptr);
+    c_ptr = deserialize_value_from_char_buffer(filtration, c_ptr);
     test_equality(filtration, st.filtration(st.find({0, 2})));
   }
-  c_ptr = deserialize_value_to_char_buffer(vertex, c_ptr);  // (0, 2) end of leaf
+  c_ptr = deserialize_value_from_char_buffer(vertex, c_ptr);  // (0, 2) end of leaf
   BOOST_CHECK(vertex == 0);
-  c_ptr = deserialize_value_to_char_buffer(vertex, c_ptr);  // (1) end of leaf
+  c_ptr = deserialize_value_from_char_buffer(vertex, c_ptr);  // (1) end of leaf
   BOOST_CHECK(vertex == 0);
-  c_ptr = deserialize_value_to_char_buffer(vertex, c_ptr);  // (2) end of leaf
+  c_ptr = deserialize_value_from_char_buffer(vertex, c_ptr);  // (2) end of leaf
   BOOST_CHECK(vertex == 0);
 
   std::size_t index = static_cast<std::size_t>(c_ptr - buffer);
@@ -390,7 +390,7 @@ BOOST_AUTO_TEST_CASE(simplex_tree_custom_deserialization_vec_to_double) {
   auto deserialize_filtration_value = [](typename target_st_type::Filtration_value& fil,
                                          const char* start) -> const char* {
     typename source_st_type::Filtration_value origin_fil;
-    const char* ptr = deserialize_value_to_char_buffer(origin_fil, start); //naive way to do it, but fine enough for a test
+    const char* ptr = deserialize_value_from_char_buffer(origin_fil, start); //naive way to do it, but fine enough for a test
     fil = origin_fil[0];
     return ptr;
   };
@@ -428,7 +428,7 @@ BOOST_AUTO_TEST_CASE(simplex_tree_custom_deserialization_double_to_vec) {
   auto deserialize_filtration_value = [](typename target_st_type::Filtration_value& fil,
                                          const char* start) -> const char* {
     typename source_st_type::Filtration_value origin_fil;
-    const char* ptr = deserialize_value_to_char_buffer(origin_fil, start);
+    const char* ptr = deserialize_value_from_char_buffer(origin_fil, start);
     fil.resize(2, 1);
     fil[0] = origin_fil;
     return ptr;

src/Simplex_tree/test/test_vector_filtration_simplex_tree.h

@@ -79,7 +79,7 @@ class Vector_filtration_value : public std::vector<int>
     return start + arg_size + type_size;
   }
 
-  friend const char* deserialize_value_to_char_buffer(Vector_filtration_value& value, const char* start)
+  friend const char* deserialize_value_from_char_buffer(Vector_filtration_value& value, const char* start)
   {
     const std::size_t type_size = sizeof(Vector_filtration_value::size_type);
     Vector_filtration_value::size_type length;