[Simplex tree] empty dummies with if constexpr

[hschreiber]

This is related to PR #976 / #817 : with vectors as filtration values, the method filtration needs to return references instead of copies now. Before the integration of C++17 to Gudhi, to handle the different options, the "dummy" options had to implement the optional method too, just trivially. For the filtration values, it means that the dummy has to return a reference too when calling filtration, forcing the addition of a dummy empty value to point to. Instead we could just empty out the dummies (except non default constructors) and put if constexpr in front of the use of optional methods. It would not only avoid useless calls, but also having to maintain them.

[mglisse]

This is related to PR #976 / #817 : with vectors as filtration values, the method filtration needs to return references instead of copies now. Before the integration of C++17 to Gudhi, to handle the different options, the "dummy" options had to implement the optional method too, just trivially. For the filtration values, it means that the dummy has to return a reference too when calling filtration, forcing the addition of a dummy empty value to point to.

I didn't check this particular case, but note that the dummy could return by value and the non-dummy return by reference, that's not forbidden. And then it may be convenient to use decltype(auto) in a function that forwards to either of those.

Instead we could just empty out the dummies (except non default constructors) and put if constexpr in front of the use of optional methods. It would not only avoid useless calls, but also having to maintain them.

We would have to see in details what the new code looks like compared to the old one. Having the dummy provide a filtration value of 0 means that a number of things "just worked", for instance computing the cohomology of a non-filtered complex, without having to spam if constexpr (store_filtration) all over the place. On the other hand, it may have silently wasted time, for instance sorting the simplices by filtration order when the order of for_each_simplex might be just as good (untested). Also, while returning 0 unnecessarily was essentially free, things may be different with vectors (or not).

[hschreiber]

I didn't check this particular case, but note that the dummy could return by value and the non-dummy return by reference, that's not forbidden. And then it may be convenient to use decltype(auto) in a function that forwards to either of those.

That is what I thought about first, but we had some weird issues because of it. I don't remember exactly what, because we corrected quite a few things that day, but using a const reference for the dummy resolved something. But I have to admit, I didn't thought about auto, so this could perhaps been another way to solve it, I would have to relook at it. But what is sure, is that the dummy will not be able to implement a Filtration_value& filtration without the const (which is really useful for multi --- the other solution would be to add an overload for assign_filtration modifying just a coordinate and not the whole vector.). And I find it conceptually weird that the dummy has to be tested when is_multi_parameter but not when store_filtration == true.

without having to spam if constexpr (store_filtration) all over the place.

I admit that the if constexpr will probably be all over the place. But I do prefer that than having to call a function which does something when it is not even supposed to be defined conceptually. But I guess that is quite subjective, that is why I wanted to discuss it.

On the other hand, it may have silently wasted time, for instance sorting the simplices by filtration order when the order of for_each_simplex might be just as good (untested). Also, while returning 0 unnecessarily was essentially free, things may be different with vectors (or not).

I agree that those are unclear... That is why I thought it would be a good idea to open the issue to remember myself to test that at some point.