A fix proposal for boudaries of a simplex python version

[VincentRouvreau]

A fix proposal for #29
Maybe overkill because it could be done easily in pure Python, but the interface would not be the same as the other iterators.

import itertools

    def get_boundaries(self, simplex):
        """This function returns a generator with the boundaries of a given N-simplex.

        :param simplex: The N-simplex, represented by a list of vertex.
        :type simplex: list of int.
        :returns:  The (simplices of the) boundaries of a simplex
        :rtype:  generator of simplices
        """
        return itertools.combinations(simplex,len(simplex)-1)

We wouldn't have :rtype: generator with tuples(simplex, filtration) like the other generators.

[mglisse]

the interface would not be the same as the other iterators

Generators and lists are both iterable. The whole point of those concepts is that you don't have to use the same type all the time. I haven't looked at the PR yet.

---

Ignore the above, I misunderstood. The main point is whether we should get a range of simplices, or a range of pairs (simplex, filtration), which is what the others do. Being consistent makes sense here. Maybe we can add a comment pointing out that if the user does not need the filtration values but just the simplices, they can also use itertools.combinations(simplex,len(simplex)-1), but I don't even think that's necessary.

[mglisse]

Maybe check also the boundary of a vertex (should be an empty list).
What happens if I do st.get_boundaries([])? (I don't think it is very important, I am just curious)

[VincentRouvreau]

I added a test on 8f9c065
st.get_boundaries([]) seg faults... I have a look on it

[VincentRouvreau]

Ok this is because st.boundary_simplex_range(st.null_simplex()) seg faults.

If I fix it in Simplex_tree_iterators.h by saying:

  template<class SimplexHandle>
  Simplex_tree_boundary_simplex_iterator(SimplexTree * st, SimplexHandle sh)
      : //last_(sh->first),
        next_(st->null_vertex()),
        sib_(nullptr),
        sh_(st->null_simplex()),
        st_(st) {
    // Only check once at the beginning instead of for every increment, as this is expensive.
    if (SimplexTree::Options::contiguous_vertices)
      GUDHI_CHECK(st_->contiguous_vertices(), "The set of vertices is not { 0, ..., n } without holes");
    if (sh != st->null_simplex()) {
      last_ = sh->first;
      Siblings * sib = st->self_siblings(sh);
      next_ = sib->parent();
      sib_ = sib->oncles();
    }

(I find template<class SimplexHandle> quite strange here)
It works fine, but now Cech complex compilation fails because of:

Simplex_tree.h:1262:26: error: conversion from ‘boost::container::flat_map<int, Gudhi::Simplex_tree_node_explicit_storage<Gudhi::Simplex_tree<> >, std::less<int>, boost::container::new_allocator<std::pair<int, Gudhi::Simplex_tree_node_explicit_storage<Gudhi::Simplex_tree<> > > > >::reverse_iterator {aka boost::intrusive::reverse_iterator<boost::container::container_detail::vec_iterator<std::pair<int, Gudhi::Simplex_tree_node_explicit_storage<Gudhi::Simplex_tree<> > >*, false> >}’ to non-scalar type ‘Gudhi::Simplex_tree<>::Simplex_handle {aka boost::container::container_detail::vec_iterator<std::pair<int, Gudhi::Simplex_tree_node_explicit_storage<Gudhi::Simplex_tree<> > >*, false>}’ requested
       for(Simplex_handle next = siblings->members().rbegin(); next != simplex; next++) {
                          ^~~~

It cannot convert a boost::intrusive::reverse_iterator<boost::container::container_detail::vec_iterator<...>> to a boost::container::container_detail::vec_iterator<...>

[mglisse]

I think it is ok to crash in the C++ code when violating a precondition (sh has to refer to a real simplex). It is just in python that crashing the interpreter is a bit too extreme. assert len(simplex) != 0 would be good enough for me, or something similar in the C++ glue.
Maybe at some point we could introduce something similar to GUDHI_CHECK, for things that we want to check even in release mode (and throw an exception if that fails) but only when used from python. I don't know exactly how much we would use it, but I think it isn't the first time I've thought of that.

[VincentRouvreau]

Hmm ok. Sounds good to me. I think I need to perform something like assert find(simplex) != null_simplex() because something not found would also seg fault.

[mglisse]

Ah, I hadn't thought of that case...

[VincentRouvreau]

Ok, I used exceptions on 458bc2d and seems better.
I also added some tests to the limits.