CGAL / cgal

The public CGAL repository, see the README below
https://github.com/CGAL/cgal#readme
Other
4.86k stars 1.38k forks source link

CGAL 5.6 Exact_field_selector problem with boost::multiprecision::float128 #7715

Closed cosurgi closed 12 months ago

cosurgi commented 1 year ago

Issue Details

Hi, I am YADE developer. In our pipeline we have an error when building with CGAL 5.6.

builds/yade-dev/trunk/lib/base/AliasCGAL.hpp:52:1:   required from here
/usr/include/CGAL/Exact_kernel_selector.h:37:75: error: invalid use of incomplete type ‘struct CGAL::internal::Exact_field_selector<boost::multiprecision::number<boost::multiprecision::backends::float128_backend, boost::multiprecision::et_off> >’
   37 |   typedef typename internal::Exact_field_selector<typename CK::RT>::Type  Exact_nt;
      |                                                                           ^~~~~~~~
In file included from /usr/include/CGAL/Number_types/internal/Exact_type_selector.h:25,
                 from /usr/include/CGAL/Exact_kernel_selector.h:26:
/usr/include/CGAL/Lazy_exact_nt.h:1341:23: note: declaration of ‘struct CGAL::internal::Exact_field_selector<boost::multiprecision::number<boost::multiprecision::backends::float128_backend, boost::multiprecision::et_off> >’
 1341 | template<class>struct Exact_field_selector;
      |                       ^~~~~~~~~~~~~~~~~~~~
/usr/include/CGAL/Exact_kernel_selector.h:38:75: error: invalid use of incomplete type ‘struct CGAL::internal::Exact_ring_selector<boost::multiprecision::number<boost::multiprecision::backends::float128_backend, boost::multiprecision::et_off> >’
   38 |   typedef typename internal::Exact_ring_selector <typename CK::RT>::Type  Exact_rt;
      |                                                                           ^~~~~~~~

The failing pipeline is for example this one We are building YADE for several high precision types, the types are listed in type RealHPLadder , depending on compilation flags different high precision types are used. This worked flawlessy with CGAL 5.5.

This RealHPLadder is then used by the AliasCGAL.hpp file to invoke CGAL Exact_field_selector where levelHP inside ERealHP<levelHP> corresponds to a high precision type inside RealHPLadder. Consequently the macro is used for a list of all high precision types, after C++ preprocessing the code in linked earlier file AliasCGAL.hpp looks like this:

namespace CGAL {
template <int levelHP> class ERealHP;
template <>
class ERealHP<1> : public Filtered_kernel_adaptor<Type_equality_wrapper<Simple_cartesian<::yade::RealHP<1>>::Base<ERealHP<1>>::Type, ERealHP<1>>, true> {
};
template <>
class ERealHP<2> : public Filtered_kernel_adaptor<Type_equality_wrapper<Simple_cartesian<::yade::RealHP<2>>::Base<ERealHP<2>>::Type, ERealHP<2>>, true> {
};
template <>
class ERealHP<3> : public Filtered_kernel_adaptor<Type_equality_wrapper<Simple_cartesian<::yade::RealHP<3>>::Base<ERealHP<3>>::Type, ERealHP<3>>, true> {
};
template <>
class ERealHP<4> : public Filtered_kernel_adaptor<Type_equality_wrapper<Simple_cartesian<::yade::RealHP<4>>::Base<ERealHP<4>>::Type, ERealHP<4>>, true> {
};
template <>
class ERealHP<8> : public Filtered_kernel_adaptor<Type_equality_wrapper<Simple_cartesian<::yade::RealHP<8>>::Base<ERealHP<8>>::Type, ERealHP<8>>, true> {
};
template <>
class ERealHP<10> : public Filtered_kernel_adaptor<Type_equality_wrapper<Simple_cartesian<::yade::RealHP<10>>::Base<ERealHP<10>>::Type, ERealHP<10>>, true> {
};
template <>
class ERealHP<20> : public Filtered_kernel_adaptor<Type_equality_wrapper<Simple_cartesian<::yade::RealHP<20>>::Base<ERealHP<20>>::Type, ERealHP<20>>, true> {
};
using Exact_Real_predicates_inexact_constructions_kernel                          = ERealHP<1>;
template <int levelHP> using Exact_RealHP_predicates_inexact_constructions_kernel = ERealHP<levelHP>;
template <> struct Triangulation_structural_filtering_traits<ERealHP<1>> {
    typedef Tag_true Use_structural_filtering_tag;
};
template <> struct Triangulation_structural_filtering_traits<ERealHP<2>> {
    typedef Tag_true Use_structural_filtering_tag;
};
template <> struct Triangulation_structural_filtering_traits<ERealHP<3>> {
    typedef Tag_true Use_structural_filtering_tag;
};
template <> struct Triangulation_structural_filtering_traits<ERealHP<4>> {
    typedef Tag_true Use_structural_filtering_tag;
};
template <> struct Triangulation_structural_filtering_traits<ERealHP<8>> {
    typedef Tag_true Use_structural_filtering_tag;
};
template <> struct Triangulation_structural_filtering_traits<ERealHP<10>> {
    typedef Tag_true Use_structural_filtering_tag;
};
template <> struct Triangulation_structural_filtering_traits<ERealHP<20>> {
    typedef Tag_true Use_structural_filtering_tag;
};
}

where ::yade::RealHP<1>> is usually a regular double type. And this line compiles without problems. Next the ::yade::RealHP<2>> is used, which corresponds to boost::multiprecision::float128 and this compilation fails. Then other higher types are used either from MPFR or boost::cpp_bin_float depending on compilation flags. They don't compile either and produce the Exact_field_selector error given at beginning.

We are tracking this issue here: https://gitlab.com/yade-dev/trunk/-/issues/319

Source Code

Currently the issue is reproduced by compiling whole YADE package: failing pipeline, you can compile YADE locally by following installation instructions. We don't have a Minimal Working Example yet.

Environment

(**) full list of flags:

-Wformat -Wformat-security -Wformat-nonliteral -Wall -Wextra -Wnarrowing -Wreturn-type -Wuninitialized -Wfloat-conversion -Wcast-align -Wdisabled-optimization -Wtrampolines -Wpointer-arith -Wswitch-bool -Wwrite-strings -Wnon-virtual-dtor -Wreturn-local-addr -Wsuggest-override -Wshadow -Wswitch-default -Wno-error=maybe-uninitialized -Wno-error=array-bounds= -Wno-error=cpp -fdce -fstack-protector-strong -fopenmp -O3 -std=gnu++17 -fPIC -ftrack-macro-expansion=0 -frounding-math -DBOOST_LOG_DYN_LINK -DFREEGLUT_VERSION_MAJOR=3 -DMAX_LOG_LEVEL=6 -DQT_NO_KEYWORDS -DSUITESPARSE_VERSION_4 -DYADE_BOOST_LOG -DYADE_COMPLEX_MP -DYADE_FEM -DYADE_LS_DEM -DYADE_ODEINT -DYADE_POTENTIAL_BLOCKS -DYADE_POTENTIAL_PARTICLES -DYADE_REAL_BIT=64 -DYADE_REAL_DEC=15 -D_minieigenHP_EXPORTS -I/home/ytest/yade/trunk -I/usr/include/python3.11 -I/usr/lib/python3/dist-packages/numpy/core/include -I/usr/include/eigen3 -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -I/usr/include/x86_64-linux-gnu/qt5 -I/usr/include/x86_64-linux-gnu/qt5/QtCore -I/usr/lib/x86_64-linux-gnu/qt5/mkspecs/linux-g++ -I/usr/include/x86_64-linux-gnu/qt5/QtWidgets -I/usr/include/x86_64-linux-gnu/qt5/QtGui -I/usr/include/x86_64-linux-gnu/qt5/QtXml -I/usr/include/x86_64-linux-gnu/qt5/QtOpenGL -I/usr/include/GL -I/usr/include/suitesparse -I/usr/lib/x86_64-linux-gnu/openmpi/include -I/usr/lib/x86_64-linux-gnu/openmpi/include/openmpi -I/usr/lib/python3/dist-packages/mpi4py/include -I/tmp/15-YADE/24-yade-kompilacja/yade/build-trunk -I/usr/include/coin -I/usr/lib/x86_64-linux-gnu/pkgconfig -DYADE_VTK -DYADE_OPENMP -DYADE_GTS -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -I/usr/include -DQGLVIEWER_FOUND -DYADE_OPENGL -DYADE_QT5 -DYADE_CGAL -DFLOW_ENGINE -DFLOW_ENGINE -DLINSOLV -DYADE_MPI -DTWOPHASEFLOW -DYADE_GL2PS -DLBM_ENGINE -DTHERMAL -DPARTIALSAT -DNDEBUG -DEIGEN_DONT_VECTORIZE -DEIGEN_DONT_ALIGN -DEIGEN_DISABLE_UNALIGNED_ARRAY_ASSERT -DCGAL_DISABLE_ROUNDING_MATH_CHECK

cosurgi commented 1 year ago

Full list of library versions:

library
boost 1.81.0
cgal 5.6
clp 1.17.6
cmake 3.27.4
coinutils 2.11.4
compiler gcc 13.2
eigen 3.4.0
freeglut 3.4.0
ipython 8.14.0
metis 5.1.0
mpc 1.3.1
mpfr 4.2.1
openblas 0.3.24
python 3.11.4-5
suitesparse 1:7.1.0
vtk 9.1.0
sloriot commented 12 months ago

Hi, thanks for the report.

#include <CGAL/Exact_predicates_inexact_constructions_kernel.h>
#include <boost/multiprecision/float128.hpp>

namespace CGAL {
template <int levelHP> class ERealHP;
template <>
class ERealHP<1> : public Filtered_kernel_adaptor<Type_equality_wrapper<Simple_cartesian<double>::Base<ERealHP<1>>::Type, ERealHP<1>>, true> {
};
template <>
class ERealHP<2> : public Filtered_kernel_adaptor<Type_equality_wrapper<Simple_cartesian<boost::multiprecision::float128>::Base<ERealHP<2>>::Type, ERealHP<2>>, true> {
};
}

int main()
{}

This does not compile with CGAL-5.5 on my machine (If I comment the boost mp part it does). Are you sure that ::yade::RealHP<2> evaluates to boost::multiprecision::float128?

cosurgi commented 12 months ago

Yes, it is boost::multiprecision::float128. However I just realized that there are more include files involved to make it work with CGAL 5.5: CgalNumTraits.hpp and RealHPEigenCgal.hpp.

I am now trying to extract it into a minimal example like the one you pasted above.

cosurgi commented 12 months ago

OK. I managed co make MWE. This compiles with CGAL 5.5 and produces this error in CGAL 5.6. However I might have a bit too much code in it. Maybe some extra definitions could be removed.

#include <cmath>
#include <CGAL/Interval_nt.h>
#include <CGAL/NT_converter.h>
#include <CGAL/number_type_basic.h>
#include <CGAL/gmpxx.h>
#include <boost/multiprecision/gmp.hpp>

#include <CGAL/Exact_predicates_inexact_constructions_kernel.h>
#include <boost/multiprecision/float128.hpp>

namespace yade {
    using Real = double;

namespace math {
    using UnderlyingReal = double;
}
}

namespace yade {
template <int L> struct SomeFloat;
template<> struct SomeFloat<1> { using type = double; };
template<> struct SomeFloat<2> { using type = boost::multiprecision::float128; };

template <int L>
using RealHP = typename SomeFloat<L>::type;
}

///////////////////////////////////////////////////////
namespace CGAL {
template <int levelHP> class RealHP_Is_valid : public CGAL::cpp98::unary_function<::yade::RealHP<levelHP>, bool> {
public:
    bool operator()(const ::yade::RealHP<levelHP>& x) const { return not isnan(x); }
};
template <int levelHP> class RealHP_Algebraic_structure_traits : public Algebraic_structure_traits_base<::yade::RealHP<levelHP>, Field_with_kth_root_tag> {
public:
    typedef Tag_false               Is_exact;
    typedef Tag_true                Is_numerical_sensitive;
    typedef ::yade::RealHP<levelHP> Type;
    class Square : public CGAL::cpp98::unary_function<Type, Type> {
    public:
        Type operator()(const Type& x) const { return pow(x, 2); }
    };
    class Sqrt : public CGAL::cpp98::unary_function<Type, Type> {
    public:
        Type operator()(const Type& x) const { return sqrt(x); }
    };
    class Kth_root : public CGAL::cpp98::binary_function<int, Type, Type> {
    public:
        Type operator()(int k, const Type& x) const
        {
            (static_cast<void>(0));
            return pow(x, static_cast<::yade::RealHP<levelHP>>(1.0) / static_cast<::yade::RealHP<levelHP>>(k));
        };
    };
};
template <int levelHP> class RealHP_embeddable_traits : public INTERN_RET::Real_embeddable_traits_base<::yade::RealHP<levelHP>, CGAL::Tag_true> {
public:
    typedef ::yade::RealHP<levelHP> Type;
    class To_interval : public CGAL::cpp98::unary_function<Type, std::pair<double, double>> {
    public:
        std::pair<double, double> operator()(const Type& x) const { return (Interval_nt<>(static_cast<double>(x)) + Interval_nt<>::smallest()).pair(); }
    };
    class Sgn : public CGAL::cpp98::unary_function<Type, CGAL::Sign> {
    public:
        CGAL::Sign operator()(const Type& x) const { return CGAL::Sign(sign(x)); }
    };
    class Is_finite : public CGAL::cpp98::unary_function<Type, bool> {
    public:
        bool operator()(const Type& x) const { return isfinite(x); }
    };
};
template <typename GMP1, typename GMP2> struct NT_converter<::yade::Real, __gmp_expr<GMP1, GMP2>> : public CGAL::cpp98::unary_function<::yade::Real, NT_converter<::yade::Real, __gmp_expr<GMP1, GMP2>>> {
    __gmp_expr<GMP1, GMP2> operator()(const ::yade::Real& a) const
    {
        mpq_t ret;
        __gmpq_init(ret);
        __gmpq_set(ret, boost::multiprecision::mpq_rational(static_cast<::yade::math::UnderlyingReal>(a)).backend().data());
        return __gmp_expr<GMP1, GMP2>(ret);
    }
};
template <> struct NT_converter<::yade::Real, boost::multiprecision::mpq_rational> : public CGAL::cpp98::unary_function<::yade::Real, NT_converter<::yade::Real, boost::multiprecision::mpq_rational>> {
    boost::multiprecision::mpq_rational operator()(const ::yade::Real& a) const { return boost::multiprecision::mpq_rational(static_cast<::yade::math::UnderlyingReal>(a)); }
};
}

///////////////////////////////////////////////////////

namespace CGAL {
template <> struct Is_valid<::yade::RealHP<2>> : public RealHP_Is_valid<2> {
};
template <> struct Algebraic_structure_traits<::yade::RealHP<2>> : public RealHP_Algebraic_structure_traits<2> {
};
template <> struct Real_embeddable_traits<::yade::RealHP<2>> : public RealHP_embeddable_traits<2> {
};

} // namespace CGAL

///////////////////////////////////////////////////////

namespace CGAL {
template <int levelHP> class ERealHP;
template <>
class ERealHP<1> : public Filtered_kernel_adaptor<Type_equality_wrapper<Simple_cartesian<double>::Base<ERealHP<1>>::Type, ERealHP<1>>, true> {
};
template <>
class ERealHP<2> : public Filtered_kernel_adaptor<Type_equality_wrapper<Simple_cartesian<boost::multiprecision::float128>::Base<ERealHP<2>>::Type, ERealHP<2>>, true> {
};
}

int main()
{}
cosurgi commented 12 months ago

I managed to make MWE a bit shorter:

#include <CGAL/Exact_predicates_inexact_constructions_kernel.h>
#include <boost/multiprecision/float128.hpp>

namespace yade {
    using Real = double;

namespace math {
    using UnderlyingReal = double;
}
}

namespace yade {
template <int L> struct SomeFloat;
template<> struct SomeFloat<1> { using type = double; };
template<> struct SomeFloat<2> { using type = boost::multiprecision::float128; };

template <int L>
using RealHP = typename SomeFloat<L>::type;
}

namespace CGAL {
template <int levelHP> class RealHP_Is_valid : public CGAL::cpp98::unary_function<::yade::RealHP<levelHP>, bool> {
public:
    bool operator()(const ::yade::RealHP<levelHP>& x) const { return not isnan(x); }
};
template <int levelHP> class RealHP_Algebraic_structure_traits : public Algebraic_structure_traits_base<::yade::RealHP<levelHP>, Field_with_kth_root_tag> {
public:
    typedef Tag_false               Is_exact;
    typedef Tag_true                Is_numerical_sensitive;
    typedef ::yade::RealHP<levelHP> Type;
    class Square : public CGAL::cpp98::unary_function<Type, Type> {
    public:
        Type operator()(const Type& x) const { return pow(x, 2); }
    };
    class Sqrt : public CGAL::cpp98::unary_function<Type, Type> {
    public:
        Type operator()(const Type& x) const { return sqrt(x); }
    };
    class Kth_root : public CGAL::cpp98::binary_function<int, Type, Type> {
    public:
        Type operator()(int k, const Type& x) const
        {
            (static_cast<void>(0));
            return pow(x, static_cast<::yade::RealHP<levelHP>>(1.0) / static_cast<::yade::RealHP<levelHP>>(k));
        };
    };
};
template <int levelHP> class RealHP_embeddable_traits : public INTERN_RET::Real_embeddable_traits_base<::yade::RealHP<levelHP>, CGAL::Tag_true> {
public:
    typedef ::yade::RealHP<levelHP> Type;
    class To_interval : public CGAL::cpp98::unary_function<Type, std::pair<double, double>> {
    public:
        std::pair<double, double> operator()(const Type& x) const { return (Interval_nt<>(static_cast<double>(x)) + Interval_nt<>::smallest()).pair(); }
    };
    class Sgn : public CGAL::cpp98::unary_function<Type, CGAL::Sign> {
    public:
        CGAL::Sign operator()(const Type& x) const { return CGAL::Sign(sign(x)); }
    };
    class Is_finite : public CGAL::cpp98::unary_function<Type, bool> {
    public:
        bool operator()(const Type& x) const { return isfinite(x); }
    };
};
}

namespace CGAL {
template <> struct Is_valid<::yade::RealHP<2>> : public RealHP_Is_valid<2> {
};
template <> struct Algebraic_structure_traits<::yade::RealHP<2>> : public RealHP_Algebraic_structure_traits<2> {
};
template <> struct Real_embeddable_traits<::yade::RealHP<2>> : public RealHP_embeddable_traits<2> {
};

} // namespace CGAL

namespace CGAL {
template <int levelHP> class ERealHP;
template <>
class ERealHP<1> : public Filtered_kernel_adaptor<Type_equality_wrapper<Simple_cartesian<::yade::RealHP<1>>::Base<ERealHP<1>>::Type, ERealHP<1>>, true> {
};
template <>
class ERealHP<2> : public Filtered_kernel_adaptor<Type_equality_wrapper<Simple_cartesian<::yade::RealHP<2>>::Base<ERealHP<2>>::Type, ERealHP<2>>, true> {
};
}

int main()
{}
mglisse commented 12 months ago

Did you try specializing Exact_field_selector (and maybe its ring counterpart if you have suitable conversions) for float128 in YADE? In order to work with float128, you already had to specialize the number traits, the conversion functor, etc. So you are the one who knows which bignum types float128 can be safely converted to. What CGAL-5.5 did probably selected mpq_rational in some cases, cpp_rational in others, maybe even leda_rational or Quotient<...>, so you needed to provide conversions from float128 to all of those to be safe, that doesn't seem great. (it is possible that more doc is needed in CGAL)

cosurgi commented 12 months ago

Can you provide some hints how to specialize Exact_field_selector? Will this work both with 5.5 and 5.6 ?

mglisse commented 12 months ago

Can you provide some hints how to specialize Exact_field_selector?

Something like

namespace CGAL::internal {
template<>struct Exact_field_selector<boost::multiprecision::float128>{typedef XXX Type;};
}

Where you replace XXX with the rational type of your choice. Look at CGAL/Number_types/internal/Exact_type_selector.h.

Will this work both with 5.5 and 5.6 ?

I think so.

cosurgi commented 12 months ago

Thank you. I added these lines:

namespace CGAL::internal {
template<>struct Exact_field_selector<boost::multiprecision::float128>{using Type=boost::multiprecision::mpq_rational;};
template<>struct Exact_ring_selector<boost::multiprecision::float128>{using Type=boost::multiprecision::mpq_rational;};
}

And now it is compiling with both CGAL 5.5 and 5.6. I will try this solution in YADE.

cosurgi commented 12 months ago

I will close this issue now, because it appears to be fixed. I will eventually reopen if there will be some problem in YADE with this solution.

cosurgi commented 12 months ago

Yes. It works in YADE now. Thank you very much :)