← Back to team overview

yade-dev team mailing list archive

Re: High precision Real. (minieigen patches)

 

In the attachment are the minimal minieigen patches:

These are the super-minimal necessary patches:

1. 50_fix_minieigen_agnostic_to_Real.patch
2. 52_fix_convertible_Scalar_types.patch
3. python3-minieigen.install

This one is not strictly necessary. It's just a bugfix. The Vector4r
was broken, because 4th argument to the constructor was missing:

4. 51_fix_Vector4r.patch

Sorry for the mixed numbering of patches, I preferred to keep the
original (old) patch numbers.


I am nicely surprised that these patches turned out to be so small.
I just learned that when compiling with #included external library
files the 'unused variable' warning is muted by g++. And so the
largest patch is not necessary. Other patches which I had here
initially were concerning the fixes of AlignedVector3. But for now we
don't use it. We have make_SSE vectorization in [4] without it.


I am really sorry for the delay. I wanted to be 100% sure that all
pipelines pass. (there were some problems with the build servers).

Anton, can you apply these patches to the minieigen package?


In a couple of days I should also submit a merge request to minieigen upstream.

The latest pipeline [1] passes with a super-minimal set of patches [2][3].

The MR 383 [1] can be merged after all the others are merged. It will
then have only couple of commits. Also then I will rearrange the
pipeline a bit, so that there are fewer asan tests, but they will be rotating.
Currently they use [5] 'debian-bullseye-SMALLEST-PATCH-minieigen' build image.

best regards
Janek


[1] https://gitlab.com/yade-dev/trunk/-/merge_requests/383/pipelines
[2] https://gitlab.com/yade-dev/docker-yade/commit/a3accab668a4896bff4e5ec1fdf0df6b741d638c
[3] https://gitlab.com/yade-dev/trunk/-/merge_requests/383/diffs?commit_id=fff0dc8fda163d57f8816e61201660cd3b6a0a38
[4] https://gitlab.com/yade-dev/trunk/-/merge_requests/362
[5] https://gitlab.com/yade-dev/docker-yade/container_registry

Janek Kozicki (yade) said:     (by the date of Mon, 27 Jan 2020 21:38:09 +0100)

> Hi Anton,
> 
> > Can we avoid it somehow? minieigen sources were in Yade tree many years
> > ago and we managed to drop them, introducing minieigen packages.
> > It would not be the best variant to put them in the source again.
> 
> Another solution is that you will include these patches [1] in the debian
> and ubuntu package until Vaclav accepts my merge request (which I
> will prepare in two or three days).
> 
> I will review these patches again to see if they can be made smaller
> than they currently are. And I will let you know by Wednesday evening.
> (tomorrow I have lectures and a meeting).
> 
> Please note that file [2] installs the source files
> in /usr/include/minieigen, they are needed. And also that's why [3]
> is necessary. To partially avoid this we could work on preparing
> `long double` and `float128` versions of minieigen package.
> But for MPFR and boost::multiprecision::cpp_bin_float these sources are
> still necessary because the precision is specified during compilation.
> 
> best regards
> Janek
> 
> BTW: you have write access to [4]
> 
> [1] https://gitlab.com/cosurgi/minieigen-real/tree/master/patches
> [2] https://gitlab.com/cosurgi/minieigen-real/blob/master/patches/python3-minieigen.install
> [3] https://gitlab.com/cosurgi/minieigen-real/blob/master/patches/54_fix_compilation_warnings.patch
> [4] https://gitlab.com/cosurgi/minieigen-real
> 
> 
> 
> 
> Anton Gladky said:     (by the date of Mon, 27 Jan 2020 21:07:15 +0100)
> 
> > Hi Janek,
> > 
> > > BTW, Anton did you notice that building debian package fails on
> > > master due to missing python-gts?
> > > https://gitlab.com/yade-dev/trunk/issues/139
> > 
> > please review this MR [1]. It fixes build-dependency problem and also
> > --help regression introduced with "--stdperformance" option.
> > 
> > > And in meantime I will include minieigen sources in yade tree, just
> > > like we did with python-gts. By this way it will work for everyone
> > > without dependency hell, until the changes are in official minieigen
> > > repository.
> > 
> > Can we avoid it somehow? minieigen sources were in Yade tree many years
> > ago and we managed to drop them, introducing minieigen packages.
> > It would not be the best variant to put them in the source again.
> > 
> > [1] https://gitlab.com/yade-dev/trunk/-/merge_requests/396
> > 
> > Regards
> > 
> > Anton
> > 
> > Am Mo., 27. Jan. 2020 um 16:26 Uhr schrieb Janek Kozicki (yade)
> > <jkozicki-yade@xxxxxxxxx>:
> > >
> > > Anton Gladky said:     (by the date of Thu, 23 Jan 2020 06:44:02)
> > >
> > > > I do really appreciate this kind of work!
> > > > But how can we rely on custom builds of the third party software?
> > > > Sorry, but this restriction will get Yade into the dependency hell.
> > >
> > > Yes, I completely agree with this premise. I will prepare merge
> > > request for Vaclav in https://github.com/eudoxos/minieigen
> > >
> > > And in meantime I will include minieigen sources in yade tree, just
> > > like we did with python-gts. By this way it will work for everyone
> > > without dependency hell, until the changes are in official minieigen
> > > repository.
> > >
> > > thanks a lot for your work.
> > >
> > > best regards
> > > Janek
> > >
> > > BTW, Anton did you notice that building debian package fails on
> > > master due to missing python-gts?
> > > https://gitlab.com/yade-dev/trunk/issues/139
> > >
> > >
> > > Janek Kozicki (yade) said:     (by the date of Mon, 13 Jan 2020 22:34:25 +0100)
> > >
> > > > Hi, the code for high precision is complete and passes all the tests.
> > > >
> > > > I am writing documentation for this now, I will mention there:
> > > > - about VTK ↔ double compatibility
> > > > - about GLViewer ↔ double compatibility
> > > > - how to build and run high precision code
> > > >
> > > > I have separated most of this work in to several "topic" merge
> > > > requests, so that you can check each of them separately. The general
> > > > idea is that "if you don't use high-precision then the old behavior
> > > > remains". Meaning that I avoid modifying existing code, only add some
> > > > sort of redirection and conversion layer that vanishes (is optimized
> > > > away) completely when HP is not used.
> > > >
> > > > There are some changes still not extracted into separate merge
> > > > requests from !366. I will continue working on this and writing documentation.
> > > >
> > > > These are ready for you to review:
> > > >
> > > > * The last remaining double to Real changes.
> > > >   https://gitlab.com/yade-dev/trunk/merge_requests/376
> > > >
> > > > * Print time spent on each of the --checks
> > > >   https://gitlab.com/yade-dev/trunk/merge_requests/375
> > > >   (a very short one, that's because I needed to make Lubrication tests faster in !366)
> > > >
> > > > * Ensure that VTK is compatibile with Real.
> > > >   https://gitlab.com/yade-dev/trunk/merge_requests/377
> > > >
> > > > * OpenGL Real compatibility
> > > >   https://gitlab.com/yade-dev/trunk/merge_requests/378
> > > >
> > > > * Another step towards enabling high precision Real
> > > >   https://gitlab.com/yade-dev/trunk/merge_requests/362
> > > >
> > > > * Enable vectorization
> > > >   https://gitlab.com/yade-dev/trunk/merge_requests/365
> > > >
> > > > They can be merged to master in any order. If any rebase conflicts
> > > > arise I will solve them quickly. Maybe !362 and !365 should be merged last,
> > > > because I tested rebasing it on each of the previous ones.
> > > >
> > > >
> > > > please tell me what you think.
> > > >
> > > > best regards
> > > > Janek
> > > >
> > > >
> > > >
> > > > --
> > > > Janek Kozicki, PhD. DSc. Arch. Assoc. Prof.
> > > > Gdańsk University of Technology
> > > > Faculty of Applied Physics and Mathematics
> > > > Department of Theoretical Physics and Quantum Information
> > > > --
> > > > http://yade-dem.org/
> > > > http://pg.edu.pl/jkozicki (click English flag on top right)
> > > >
> > > > _______________________________________________
> > > > Mailing list: https://launchpad.net/~yade-dev
> > > > Post to     : yade-dev@xxxxxxxxxxxxxxxxxxx
> > > > Unsubscribe : https://launchpad.net/~yade-dev
> > > > More help   : https://help.launchpad.net/ListHelp
> > >
> > >
> > > --
> > > --
> > > Janek Kozicki, PhD. DSc. Arch. Assoc. Prof.
> > > Gdańsk University of Technology
> > > Faculty of Applied Physics and Mathematics
> > > Department of Theoretical Physics and Quantum Information
> > > --
> > > http://yade-dem.org/
> > > http://pg.edu.pl/jkozicki (click English flag on top right)
> 
> 
> -- 
> --
> Janek Kozicki, PhD. DSc. Arch. Assoc. Prof.
> Gdańsk University of Technology
> Faculty of Applied Physics and Mathematics
> Department of Theoretical Physics and Quantum Information
> --
> http://yade-dem.org/
> http://pg.edu.pl/jkozicki (click English flag on top right)
> 
> _______________________________________________
> Mailing list: https://launchpad.net/~yade-dev
> Post to     : yade-dev@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~yade-dev
> More help   : https://help.launchpad.net/ListHelp


-- 
--
Janek Kozicki, PhD. DSc. Arch. Assoc. Prof.
Gdańsk University of Technology
Faculty of Applied Physics and Mathematics
Department of Theoretical Physics and Quantum Information
--
http://yade-dem.org/
http://pg.edu.pl/jkozicki (click English flag on top right)
>From ce47d324969dcdfc3b472f5ee515fedbc0d05015 Mon Sep 17 00:00:00 2001
From: Janek Kozicki <jkozicki-yade@xxxxxxxxx>
Date: Thu, 19 Dec 2019 21:22:06 +0100
Subject: [PATCH] [minieigen edit] Make sure minieigen is agnostic to Real
 typedef.

---
 src/common.hpp   | 7 +++++++
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/src/common.hpp b/src/common.hpp
index b45613a..bdf021b 100644
--- a/src/common.hpp
+++ b/src/common.hpp
@@ -1,8 +1,10 @@
 #pragma once
 // common types, funcs, includes; should be included by all other files
 
+#ifndef MINIEIGEN_OVERRIDE
 /* change to float for single-precision */
 typedef double Real;
+#endif
 
 // BEGIN workaround for
 // * http://eigen.tuxfamily.org/bz/show_bug.cgi?id=528
@@ -20,6 +22,7 @@ typedef double Real;
 
 #include<unsupported/Eigen/AlignedVector3>
 
+#ifndef MINIEIGEN_OVERRIDE
 // integral type for indices, to avoid compiler warnings with int
 typedef Eigen::Matrix<int,1,1>::Index Index;
 
@@ -57,6 +60,7 @@ typedef Eigen::AlignedBox<Real,2> AlignedBox2r;
 	typedef Eigen::Matrix<complex<Real>,Eigen::Dynamic,Eigen::Dynamic> MatrixXcr;
 #endif
 
+#endif
 
 #include<string>
 using std::string;
@@ -71,6 +75,7 @@ namespace py=boost::python;
 using boost::lexical_cast;
 #include<boost/static_assert.hpp>
 
+#ifndef MINIEIGEN_OVERRIDE
 /**** double-conversion helpers *****/
 #include"double-conversion/double-conversion.h"
 
@@ -131,6 +136,7 @@ static inline string num_to_string(const double& num, int pad=0){ return doubleT
 	}
 #endif
 
+#endif
 
 /*** getters and setters with bound guards ***/
 static inline void IDX_CHECK(Index i,Index MAX){ if(i<0 || i>=MAX) { PyErr_SetString(PyExc_IndexError,("Index "+lexical_cast<string>(i)+" out of range 0.." + lexical_cast<string>(MAX-1)).c_str()); py::throw_error_already_set(); } }
>From 48ab1423fb34f5503a356e15ee465887d12138d9 Mon Sep 17 00:00:00 2001
From: Janek Kozicki <jkozicki-yade@xxxxxxxxx>
Date: Sat, 21 Dec 2019 16:37:46 +0100
Subject: [PATCH] [minieigen edit] Ensure that all operators work correctly
 with convertible Scalar types.

---
 src/visitors.hpp | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/src/visitors.hpp b/src/visitors.hpp
index f05b507..53241fd 100644
--- a/src/visitors.hpp
+++ b/src/visitors.hpp
@@ -98,11 +98,16 @@ class MatrixBaseVisitor: public py::def_visitor<MatrixBaseVisitor<MatrixBaseT> >
 	static MatrixBaseT __iadd__(MatrixBaseT& a, const MatrixBaseT& b){ a+=b; return a; };
 	static MatrixBaseT __isub__(MatrixBaseT& a, const MatrixBaseT& b){ a-=b; return a; };
 
-	template<typename Scalar2> static MatrixBaseT __mul__scalar(const MatrixBaseT& a, const Scalar2& scalar){ return a*scalar; }
-	template<typename Scalar2> static MatrixBaseT __imul__scalar(MatrixBaseT& a, const Scalar2& scalar){ a*=scalar; return a; }
-	template<typename Scalar2> static MatrixBaseT __rmul__scalar(const MatrixBaseT& a, const Scalar2& scalar){ return a*scalar; }
-	template<typename Scalar2> static MatrixBaseT __div__scalar(const MatrixBaseT& a, const Scalar2& scalar){ return a/scalar; }
-	template<typename Scalar2> static MatrixBaseT __idiv__scalar(MatrixBaseT& a, const Scalar2& scalar){ a/=scalar; return a; }
+	template<typename Scalar2> static typename boost::enable_if <std::is_convertible<Scalar2,const Scalar&>,MatrixBaseT>::type __mul__scalar(const MatrixBaseT& a, const Scalar2& scalar){ return a*scalar; }
+	template<typename Scalar2> static typename boost::disable_if<std::is_convertible<Scalar2,const Scalar&>,MatrixBaseT>::type __mul__scalar(const MatrixBaseT& a, const Scalar2& scalar){ return a*static_cast<Scalar>(scalar); }
+	template<typename Scalar2> static typename boost::enable_if <std::is_convertible<Scalar2,const Scalar&>,MatrixBaseT>::type __imul__scalar(MatrixBaseT& a, const Scalar2& scalar){ a*=scalar; return a; }
+	template<typename Scalar2> static typename boost::disable_if<std::is_convertible<Scalar2,const Scalar&>,MatrixBaseT>::type __imul__scalar(MatrixBaseT& a, const Scalar2& scalar){ a*=static_cast<Scalar>(scalar); return a; }
+	template<typename Scalar2> static typename boost::enable_if <std::is_convertible<Scalar2,const Scalar&>,MatrixBaseT>::type __rmul__scalar(const MatrixBaseT& a, const Scalar2& scalar){ return a*scalar; }
+	template<typename Scalar2> static typename boost::disable_if<std::is_convertible<Scalar2,const Scalar&>,MatrixBaseT>::type __rmul__scalar(const MatrixBaseT& a, const Scalar2& scalar){ return a*static_cast<Scalar>(scalar); }
+	template<typename Scalar2> static typename boost::enable_if <std::is_convertible<Scalar2,const Scalar&>,MatrixBaseT>::type __div__scalar(const MatrixBaseT& a, const Scalar2& scalar){ return a/scalar; }
+	template<typename Scalar2> static typename boost::disable_if<std::is_convertible<Scalar2,const Scalar&>,MatrixBaseT>::type __div__scalar(const MatrixBaseT& a, const Scalar2& scalar){ return a/static_cast<Scalar>(scalar); }
+	template<typename Scalar2> static typename boost::enable_if <std::is_convertible<Scalar2,const Scalar&>,MatrixBaseT>::type __idiv__scalar(MatrixBaseT& a, const Scalar2& scalar){ a/=scalar; return a; }
+	template<typename Scalar2> static typename boost::disable_if<std::is_convertible<Scalar2,const Scalar&>,MatrixBaseT>::type __idiv__scalar(MatrixBaseT& a, const Scalar2& scalar){ a/=static_cast<Scalar>(scalar); return a; }
 
 	template<typename Scalar, class PyClass> static	void visit_reductions_noncomplex(PyClass& cl, typename boost::enable_if_c<Eigen::NumTraits<Scalar>::IsComplex >::type* dummy = 0){ /* do nothing*/ }
 	template<typename Scalar, class PyClass> static	void visit_reductions_noncomplex(PyClass& cl, typename boost::disable_if_c<Eigen::NumTraits<Scalar>::IsComplex >::type* dummy = 0){
-- 
2.20.1

src/*pp usr/include/minieigen/
>From e60a6169c457dbf62b130e896245ee55061706f9 Mon Sep 17 00:00:00 2001
From: Janek Kozicki <jkozicki-yade@xxxxxxxxx>
Date: Fri, 6 Dec 2019 21:54:48 +0100
Subject: [PATCH] [minieigen edit] fix Vector4r

---
 src/expose-vectors.cpp | 3 ---
 src/visitors.hpp       | 2 +-
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/src/expose-vectors.cpp b/src/expose-vectors.cpp
index 0123512..565f70d 100644
--- a/src/expose-vectors.cpp
+++ b/src/expose-vectors.cpp
@@ -13,12 +13,9 @@ void expose_vectors(){
 		.def(VectorVisitor<Vector6i>())
 	;
 
-	// broken
-	#if 0
 		py::class_<Vector4r>("Vector4","4-dimensional float vector.\n\nSupported operations (``f`` if a float/int, ``v`` is a Vector3): ``-v``, ``v+v``, ``v+=v``, ``v-v``, ``v-=v``, ``v*f``, ``f*v``, ``v*=f``, ``v/f``, ``v/=f``, ``v==v``, ``v!=v``.\n\nImplicit conversion from sequence (list, tuple, ...) of 4 floats.\n\nStatic attributes: ``Zero``, ``Ones``.",py::init<>())
 			.def(VectorVisitor<Vector4r>())
 		;
-	#endif
 
 	py::class_<Vector3r>("Vector3","3-dimensional float vector.\n\nSupported operations (``f`` if a float/int, ``v`` is a Vector3): ``-v``, ``v+v``, ``v+=v``, ``v-v``, ``v-=v``, ``v*f``, ``f*v``, ``v*=f``, ``v/f``, ``v/=f``, ``v==v``, ``v!=v``, plus operations with ``Matrix3`` and ``Quaternion``.\n\nImplicit conversion from sequence (list, tuple, ...) of 3 floats.\n\nStatic attributes: ``Zero``, ``Ones``, ``UnitX``, ``UnitY``, ``UnitZ``.",py::init<>())
 		.def(VectorVisitor<Vector3r>())
diff --git a/src/visitors.hpp b/src/visitors.hpp
index 985b0f5..f05b507 100644
--- a/src/visitors.hpp
+++ b/src/visitors.hpp
@@ -217,7 +217,7 @@ class VectorVisitor: public py::def_visitor<VectorVisitor<VectorT> >{
 	// 4-vector
 	template<typename VectorT2, class PyClass> static void visit_special_sizes(PyClass& cl, typename boost::enable_if_c<VectorT2::RowsAtCompileTime==4>::type* dummy=0){
 		cl
-		.def(py::init<typename VectorT2::Scalar,typename VectorT2::Scalar,typename VectorT2::Scalar>((py::arg("v0"),py::arg("v1"),py::arg("v2"),py::arg("v3"))))
+		.def(py::init<typename VectorT2::Scalar,typename VectorT2::Scalar,typename VectorT2::Scalar,typename VectorT2::Scalar>((py::arg("v0"),py::arg("v1"),py::arg("v2"),py::arg("v3"))))
 		;
 	}
 
-- 
2.20.1


Follow ups

References