Add a kernel that stores objects twice: with Intervals and Gmpq

[afabri]

Summary of Changes

Add a kernel and a numbertype that stores interval approximations for Gmqs.

TODO:

• Go through comments 'AF: '
• Remove initialization of the infinite vertex in T3
• Bench

Release Management.

• Affected package(s): Kernel_23
• License and copyright ownership: GF

[mglisse]

Ah, yes, that's a good idea, I did think about it when I noticed how much time a filtered rational kernel spends converting the same rational to an interval again and again.

[sloriot]

TODO: Once all tests pass, add specialization of the construction wrapper to use approx input when possible (to avoid non needed conversion to interval).

[afabri]

TODO: Once all tests pass, add specialization of the construction wrapper to use approx input when possible (to avoid non needed conversion to interval).

After the exact construction we create the approximation. However, we have "constructions" such as Construct_source_2(Segment_2) where we can just take the approximation of the point instead of regenerating it again from the exact result. Question: Isn't it enough to distinguish between result types that are copies and result types that are references?

[MaelRL]

@sloriot @afabri

2490190 and feae9aa need reviewing.

Former is to get CGAL::Iso_rectangle_2<FRK>::Rep::first_type and CGAL::Getter to play nice together.

Latter is because const CGAL::Getter<Args>::first_type&... gives a const& and then it doesn't match the specific overloads for result_of, for example for Construct_vertex_2 which by default has result type const Point_2&, but if matched with (Iso_rectangle_2, int), it becomes simply Point_2. So, result_of<CV2(const Iso_rectangle_2&, int)> won't see Point_2butconst Point_2&and then you haveconst&` of temporaries.

[MaelRL]

I have removed the hack from T3 which travis was complaining about, but it's inconsequential to what you're trying to do so nothing will change.

Even when FRK is lazy (which you have to enable with the macro CGAL_LAZY_FILTERED_RATIONAL_KERNEL), it doesn't use Handle.h so it doesn't have the for_compact_container() that is required from a type T used in a compact container.

Adding Compact_container_base as a base of FRK's objects will solve that:

--- a/Filtered_kernel/include/CGAL/Filtered_rational_kernel.h
+++ b/Filtered_kernel/include/CGAL/Filtered_rational_kernel.h
@@ -18,6 +18,7 @@
 #include <CGAL/Filtered_rational.h>
 
 #include <CGAL/Cartesian_converter.h>
+#include <CGAL/Compact_container.h>
 #include <CGAL/Filtered_kernel/Cartesian_coordinate_iterator_2.h>
 #include <CGAL/Filtered_kernel/Cartesian_coordinate_iterator_3.h>
 #include <CGAL/intersections.h>
Stage this hunk [y,n,q,a,d,j,J,g,/,e,?]? y
@@ -57,7 +58,7 @@ struct Infimum_to_double
 
 template <typename T1, typename T2>
 struct Approximate_exact_pair
-  : public std::pair<T1, T2>
+  : public std::pair<T1, T2>, public CGAL::Compact_container_base
 {
   typedef Approximate_exact_pair<T1, T2>          Self;
   typedef std::pair<T1, T2>                       Base;

but I don't know if it's the best way to go at it as I am not super familiar with the compact container inner workings.

[gdamiand]

Thanks @MaelRL , will try.

[gdamiand]

First preliminary tests with my program that computes arrangement of segments, on two datasets:

---

File1:

EPECK: 9.61331 seconds
FRK: 68.5651 seconds

second step:
sequential: 10,7 seconds
in parallel: 7,3962 seconds

File2:

EPECK: 53.8384 seconds
FRK: 302.304 seconds

second step:
sequential: 47,622 seconds
in parallel: 38,873 seconds

---

My algorithm has 3 steps. In this test, I only parallelized the second step. It works with FRK while I have a core dump with EPECK in the destructor (corrupted double-linked list).

I can try more tests if you want; and I can try to re-add the parallel version for my second step if you think it is interesting.

Note that cgal arrangement does not compile with this kernel, nor my basic viewer.

Hope this help.

[MaelRL]

FRK might have such a bad runtime because of Compute_x/y/z and similar constructions.

[gdamiand]

I mainly use the following predicates:

• do_intersect and intersection (between pair of segments, and one segment and one rectangle)
• has_on_unbounded_side, bounded_side, has_on, y_at_x

[mglisse]

FRK might have such a bad runtime because of Compute_x/y/z and similar constructions.

Couldn't that return a proxy instead of the heavy object?

But in any case, FRK is supposed to be significantly slower than Epeck, that was the whole point of the lazy kernel...

Adding Compact_container_base as a base of FRK's objects will solve that:

If we start adding various things in Compact containers, I guess an FRK segment should look for a pointer in the FRK point, which should look in the exact point, which should look in its first coordinate, which should look in its numerator, etc, stopping whenever there is already a pointer available (and avoiding shared objects?).

[MaelRL]

FRK might have such a bad runtime because of Compute_x/y/z and similar constructions.

Couldn't that return a proxy instead of the heavy object?

I don't really see how I could return a proxy. With the current definition, FRK::Point is pair<Ak::Point, EK::Point> and FRK::FT is pair<AK::FT, EK::FT>, so when you call Compute_x_3 you are pretty much forced to construct the pair. When this type of function is called within a predicate or a "bigger" construction, things are however fine because predicates/constructions are filtered hence you first extract AK/EK::Point. However, raw calls to Compute_x/y/z_3 and similar normally cheap constructions cost here a lot.

But in any case, FRK is supposed to be significantly slower than Epeck, that was the whole point of the lazy kernel...

Adding Compact_container_base as a base of FRK's objects will solve that:

If we start adding various things in Compact containers, I guess an FRK segment should look for a pointer in the FRK point, which should look in the exact point, which should look in its first coordinate, which should look in its numerator, etc, stopping whenever there is already a pointer available (and avoiding shared objects?).

I see, thanks.

[mglisse]

I don't really see how I could return a proxy.

You could return an object that contains just pointers to the approximate and exact numbers, and has a conversion operator to a true FT. If this object provides exact, to_interval, etc, in some cases you may avoid the conversion to FT (in less lucky cases you may end up converting several times to FT...). It depends how the result of Compute_x_3 is used.

[MaelRL]

I thought about this, but then you need a mechanism to know whether your pointers are still valid because you could have a function that constructs a point and returns its x.(). Maybe you can still track this, but it rather feels like the design of this kernel's objects as pair<ak, ek> was just too flawed from the start.

[mglisse]

I thought about this, but then you need a mechanism to know whether your pointers are still valid because you could have a function that constructs a point and returns its x.().

Same for kernels where x() returns a reference?

[MaelRL]

Sure, if that function returns a reference; but if you return a copy, you are still going to have a pair with a pointer to a deleted object whereas it is safe for other kernels.

[mglisse]

So the case that works with references but not this shallow pair is

auto x = get_temporary_point().x();

If I write FT instead of auto, it is safe. If I write auto const& the reference case is broken as well.

[MaelRL]

My point was something like:

FT compute_x_for_some_reason() { 
  Point p = construct_some_point();
  return p.x();
}

If you have a shallow pair, your FT will be pointing to a deleted object, even with the return by copy.

[mglisse]

Since the return type is explicitly FT, which is a heavy pair, the shallow pair returned by p.x() gets immediately converted to a heavy pair. It doesn't save anything, but it seems safe to me. The issue would be if the function returned auto (or decltype(auto)).