From 51ce9b513116f5fed2b4dc109f0b52595a2cd538 Mon Sep 17 00:00:00 2001 From: vrouvrea Date: Tue, 10 Apr 2018 14:53:20 +0000 Subject: Code review : Cech_blocker was hardcoding double replace point_iterator function in a get_point const function that returns a Point InputPointRange description Cech blocker is now templated with Cech complex, which is no more included. Deep copy of the point cloud on Cech complex ctor git-svn-id: svn+ssh://scm.gforge.inria.fr/svnroot/gudhi/branches/cechcomplex_vincent@3365 636b058d-ea47-450e-bf9e-a15bfbe3eedb Former-commit-id: 93e3cf3fe61290b88a0714b9e55ad80e01a34f36 --- .../example/cech_complex_example_from_points.cpp | 1 - src/Cech_complex/include/gudhi/Cech_complex.h | 39 ++++++++++++++-------- .../include/gudhi/Cech_complex_blocker.h | 15 +++------ src/Cech_complex/test/test_cech_complex.cpp | 26 +++++---------- 4 files changed, 38 insertions(+), 43 deletions(-) diff --git a/src/Cech_complex/example/cech_complex_example_from_points.cpp b/src/Cech_complex/example/cech_complex_example_from_points.cpp index 3b889d56..2b36e33c 100644 --- a/src/Cech_complex/example/cech_complex_example_from_points.cpp +++ b/src/Cech_complex/example/cech_complex_example_from_points.cpp @@ -1,6 +1,5 @@ #include #include -#include #include #include diff --git a/src/Cech_complex/include/gudhi/Cech_complex.h b/src/Cech_complex/include/gudhi/Cech_complex.h index 52f03d6b..abad0c21 100644 --- a/src/Cech_complex/include/gudhi/Cech_complex.h +++ b/src/Cech_complex/include/gudhi/Cech_complex.h @@ -54,11 +54,20 @@ namespace cech_complex { template class Cech_complex { private: + // Required by compute_proximity_graph using Vertex_handle = typename SimplicialComplexForProximityGraph::Vertex_handle; using Filtration_value = typename SimplicialComplexForProximityGraph::Filtration_value; using Proximity_graph = Gudhi::Proximity_graph; - using Point_iterator = typename boost::range_const_iterator::type; - using Point= typename std::iterator_traits::value_type; + + // Retrieve Coordinate type from InputPointRange + using Point_from_range_iterator = typename boost::range_const_iterator::type; + using Point_from_range = typename std::iterator_traits::value_type; + using Coordinate_iterator = typename boost::range_const_iterator::type; + using Coordinate= typename std::iterator_traits::value_type; + + public: + // Point and Point_cloud type definition + using Point = std::vector; using Point_cloud = std::vector; public: @@ -67,13 +76,20 @@ class Cech_complex { * @param[in] points Range of points. * @param[in] max_radius Maximal radius value. * - * \tparam InputPointRange must be a range for which `std::begin()` and `std::end()` methods return input - * iterators on a point. `std::begin()` and `std::end()` methods are also required for a point. + * \tparam InputPointRange must be a range of Point. Point must be a range of copyable Cartesian coordinates. * */ Cech_complex(const InputPointRange& points, Filtration_value max_radius) - : max_radius_(max_radius), - point_cloud_(std::begin(points), std::end(points)) { + : max_radius_(max_radius) { + // Point cloud deep copy + auto points_begin_itr = std::begin(points); + auto points_end_itr = std::end(points); + + point_cloud_.reserve(points_end_itr - points_begin_itr); + for (auto point_itr = points_begin_itr; point_itr < points_end_itr; point_itr++) { + point_cloud_.push_back(Point(std::begin(*point_itr), std::end(*point_itr))); + } + cech_skeleton_graph_ = Gudhi::compute_proximity_graph(point_cloud_, max_radius_, @@ -97,7 +113,7 @@ class Cech_complex { complex.insert_graph(cech_skeleton_graph_); // expand the graph until dimension dim_max complex.expansion_with_blockers(dim_max, - Cech_blocker(&complex, this)); + Cech_blocker(&complex, this)); } /** @return max_radius value given at construction. */ @@ -106,13 +122,10 @@ class Cech_complex { } /** @param[in] vertex Point position in the range. - * @return A const iterator on the point. - * @exception std::out_of_range In debug mode, if point position in the range is out. + * @return The point. */ - typename InputPointRange::const_iterator point_iterator(std::size_t vertex) const { - GUDHI_CHECK((std::begin(point_cloud_) + vertex) < std::end(point_cloud_), - std::out_of_range("Cech_complex::point - simplicial complex is not empty")); - return (std::begin(point_cloud_) + vertex); + const Point& get_point(Vertex_handle vertex) const{ + return point_cloud_[vertex]; } private: diff --git a/src/Cech_complex/include/gudhi/Cech_complex_blocker.h b/src/Cech_complex/include/gudhi/Cech_complex_blocker.h index ab56c10d..2ecef9cf 100644 --- a/src/Cech_complex/include/gudhi/Cech_complex_blocker.h +++ b/src/Cech_complex/include/gudhi/Cech_complex_blocker.h @@ -23,7 +23,6 @@ #ifndef CECH_COMPLEX_BLOCKER_H_ #define CECH_COMPLEX_BLOCKER_H_ -#include // Cech_blocker is using a pointer on Gudhi::cech_complex::Cech_complex #include // for Gudhi::Minimal_enclosing_ball_radius #include @@ -34,10 +33,6 @@ namespace Gudhi { namespace cech_complex { -// Just declaring Cech_complex class because used and not yet defined. -template -class Cech_complex; - /** \internal * \class Cech_blocker * \brief Cech complex blocker. @@ -52,14 +47,13 @@ class Cech_complex; * * \tparam InputPointRange is required by the pointer on Chech_complex for type definition. */ -template +template class Cech_blocker { private: - using Point = std::vector; - using Point_cloud = std::vector; + using Point_cloud = typename Cech_complex::Point_cloud; + using Simplex_handle = typename SimplicialComplexForCech::Simplex_handle; using Filtration_value = typename SimplicialComplexForCech::Filtration_value; - using Cech_complex = Gudhi::cech_complex::Cech_complex; public: /** \internal \brief Cech complex blocker operator() - the oracle - assigns the filtration value from the simplex @@ -69,8 +63,7 @@ class Cech_blocker { bool operator()(Simplex_handle sh) { Point_cloud points; for (auto vertex : sc_ptr_->simplex_vertex_range(sh)) { - points.push_back(Point(cc_ptr_->point_iterator(vertex)->begin(), - cc_ptr_->point_iterator(vertex)->end())); + points.push_back(cc_ptr_->get_point(vertex)); #ifdef DEBUG_TRACES std::cout << "#(" << vertex << ")#"; #endif // DEBUG_TRACES diff --git a/src/Cech_complex/test/test_cech_complex.cpp b/src/Cech_complex/test/test_cech_complex.cpp index 8658729b..5ca25db4 100644 --- a/src/Cech_complex/test/test_cech_complex.cpp +++ b/src/Cech_complex/test/test_cech_complex.cpp @@ -78,11 +78,8 @@ BOOST_AUTO_TEST_CASE(Cech_complex_for_documentation) { GUDHI_TEST_FLOAT_EQUALITY_CHECK(cech_complex_for_doc.max_radius(), max_radius); std::size_t i = 0; for (; i < points.size(); i++) { - BOOST_CHECK(points[i] == *(cech_complex_for_doc.point_iterator(i))); + BOOST_CHECK(points[i] == cech_complex_for_doc.get_point(i)); } -#ifdef GUDHI_DEBUG - BOOST_CHECK_THROW (cech_complex_for_doc.point_iterator(i+1), std::out_of_range); -#endif // GUDHI_DEBUG const int DIMENSION_1 = 1; Simplex_tree st; @@ -137,8 +134,7 @@ BOOST_AUTO_TEST_CASE(Cech_complex_for_documentation) { Point_cloud points012; for (std::size_t vertex = 0; vertex <= 2; vertex++) { - points012.push_back(Point(cech_complex_for_doc.point_iterator(vertex)->begin(), - cech_complex_for_doc.point_iterator(vertex)->end())); + points012.push_back(cech_complex_for_doc.get_point(vertex)); } std::size_t dimension = points[0].end() - points[0].begin(); Min_sphere ms012(dimension, points012.begin(),points012.end()); @@ -149,12 +145,9 @@ BOOST_AUTO_TEST_CASE(Cech_complex_for_documentation) { GUDHI_TEST_FLOAT_EQUALITY_CHECK(f012, std::sqrt(ms012.squared_radius())); Point_cloud points1410; - points1410.push_back(Point(cech_complex_for_doc.point_iterator(1)->begin(), - cech_complex_for_doc.point_iterator(1)->end())); - points1410.push_back(Point(cech_complex_for_doc.point_iterator(4)->begin(), - cech_complex_for_doc.point_iterator(4)->end())); - points1410.push_back(Point(cech_complex_for_doc.point_iterator(10)->begin(), - cech_complex_for_doc.point_iterator(10)->end())); + points1410.push_back(cech_complex_for_doc.get_point(1)); + points1410.push_back(cech_complex_for_doc.get_point(4)); + points1410.push_back(cech_complex_for_doc.get_point(10)); Min_sphere ms1410(dimension, points1410.begin(),points1410.end()); Simplex_tree::Filtration_value f1410 = st2.filtration(st2.find({1, 4, 10})); @@ -163,12 +156,9 @@ BOOST_AUTO_TEST_CASE(Cech_complex_for_documentation) { GUDHI_TEST_FLOAT_EQUALITY_CHECK(f1410, std::sqrt(ms1410.squared_radius())); Point_cloud points469; - points469.push_back(Point(cech_complex_for_doc.point_iterator(4)->begin(), - cech_complex_for_doc.point_iterator(4)->end())); - points469.push_back(Point(cech_complex_for_doc.point_iterator(6)->begin(), - cech_complex_for_doc.point_iterator(6)->end())); - points469.push_back(Point(cech_complex_for_doc.point_iterator(9)->begin(), - cech_complex_for_doc.point_iterator(9)->end())); + points469.push_back(cech_complex_for_doc.get_point(4)); + points469.push_back(cech_complex_for_doc.get_point(6)); + points469.push_back(cech_complex_for_doc.get_point(9)); Min_sphere ms469(dimension, points469.begin(),points469.end()); Simplex_tree::Filtration_value f469 = st2.filtration(st2.find({4, 6, 9})); -- cgit v1.2.3