Replace Eigen pointers with iterators

[TolisChal]

Replace eigen pointers in hpolytope.hpp and in the implementations of largest inscribed ball and elipsoid methods.

[shub-garg]

Hello if nobody is working on it, I would like to contribute to this issue! I am new to open source and trying to work my way to GSOC.

[vissarion]

@shub-garg nobody is working on it, please feel free to give it a try

[shub-garg]

Can you please guide me a little as to what exactly I need to do in this? As I said, I am new to all this, trying to learn.

[vissarion]

the issues of the library is probably not the best place for this, please join our chat on gitter https://gitter.im/GeomScale/community?utm_source=share-link&utm_medium=link&utm_campaign=share-link, also have a look at this tutorial https://github.com/GeomScale/volume_approximation/blob/develop/CONTRIBUTING.md

[vaithak]

Hi, I am just curious why do you want to replace the pointers? I am asking because firstly, Eigen doesn't provide begin() or end() methods for its matrix and vector class, also I read this answer: https://stackoverflow.com/a/16287999, which benchmarks different ways of traversing EIgen matrix and the one using pointer and .data() method is the fastest.

Please correct me if I am wrong or if I misunderstood something.

[vissarion]

@vaithak thanks for your search. Iterators are safer and less error-prone than pointers.

First, eigen supports STL compatible iterators https://eigen.tuxfamily.org/dox-devel/group__TutorialSTL.html since 3.4.

Second, it is not clear to me whether pointers are faster than iterators since the benchmarks in https://stackoverflow.com/a/16287999 are >6 years old and does not contain important information about reproducibility (e.g. compiler, platform). But if you are interested to provide your own benchmarks here we would be happy to discuss them.

[vaithak]

Thanks @vissarion for your response. Does the repository contains the updated version (3.4) of Eigen in external folder ? I don't think so, as the commit history shows that Eigen folder was updated 2 years ago, so should I update it ? If yes, then I think I should also give #114 a try, as it requires removing manual updating of external libraries.

[vissarion]

Yes, I think it makes more sense to go directly to https://github.com/GeomScale/volume_approximation/issues/114 (or part of it) by removing eigen from external and add some cmake commands that allow the user to download and/or use the system installed eigen.

[vaithak]

@vissarion @TolisChal , I think we should wait for a stable release of Eigen 3.4 for this. I tried using the 3.4.0-rc1 to get the features of iterators in Eigen, but some of the test cases are failing even without any new code changes.

[himanshuParashar0101]

we can replace the data flow of eagens

[vaithak]

@himanshuParashar0101 yup, this issue can be worked on now as we have upgraded eigen's version to 3.4.0 in #186.

[bsarthak16]

hi can anybody explain me what exactly needs to be done in this problem as i am interested in solving it

[vgnecula]

Hi! Is anybody working on this right now?

[vfisikop]

No, as far as I know.