| Re: [eigen] Specializing max_coeff_visitor for some number types |
[ Thread Index |
Date Index
| More lists.tuxfamily.org/eigen Archives
]
On 19.05.2014 22:14, Marc Glisse wrote:
On Mon, 19 May 2014, Christoph Hertzberg wrote:
On 18.05.2014 15:44, Marc Glisse wrote:
I changed the CGAL testcase because it is important that only 0 have a
score of 0 (the interval [0,1] should have a non-zero score). It is not
optimal (I would like [0,1]>[0,0] but [1,2]<[1,1]) but good enough (some
Ok, using [0,1] as pivot may be better than [-0,0] because the inverse
is [1,inf] as opposed to [-inf,inf]. But I think [-1,1] will be worse
than [0,0] in general.
That's not my reason: if we pick [0,0] as the "best", eigen will believe
the column / sub-matrix is 0 and not use this coeff as a pivot (the
inverse doesn't matter). So I need to make sure I don't return a score
of 0 unless I am sure all coeffs are [0,0]. If I pick [0,1] as the best,
the test "== Score(0)" will throw (default behavior of CGAL intervals)
and restart the whole computation with an exact number type, which seems
fine to me.
I still think your matrix is "possibly zero" as soon as a zero is inside
all intervals -- no matter if these intervals are [0,0] or [0,1000] or
[-1000,1000].
But that's completely your decision as it will not affect the patch.
I agree that [0,1000] is somewhat better than [0,0], though.
Some things about your patch I'm not sure about:
* Introducing a user-level function "cwiseScore" which I doubt will
have a purpose aside from pivoting.
I did it this way because I could just copy the cwiseAbs function, I am
not familiar enough with the code to understand how it is working. Is
there a convenient way to achieve the same thing without introducing a
user-level function? (I'll see if I can find one, but a hint would be
welcome). We can also not document the function (or mark the
documentation as internal or something) if that helps.
You can directly call MatrixBase::unaryExpr:
http://eigen.tuxfamily.org/dox-devel/classEigen_1_1MatrixBase.html#a23fc4bf97168dee2516f85edcfd4cfe7
Simply pass a scalar_score_coeff_op object:
biggest_in_corner = m_lu.bottomRightCorner(rows-k, cols-k)
.unaryExpr(scalar_score_coeff_op<Scalar>())
.maxCoeff(&row_of_biggest_in_corner, &col_of_biggest_in_corner);
should work (not tested).
* Would there be a way to avoid the extra coeff-access and abs()
calculation in case score(Scalar)==abs(Scalar)? I guess the
performance impact is very minor, but it's avoidable.
Sure, just a bit of meta-programming, I'll do that when I get the time.
I thought the extra ugliness might be taken against the patch, so I
didn't do it in the first version.
I don't know what would be the least ugly and least intrusive way. I'm
afraid that
internal::is_same<scalar_abs_op, scalar_score_coeff_op>::value
will not work. Maybe add another flag to
functor_traits<scalar_score_coeff_op<Scalar> >?
I will apply your patch (as soon as it's finished), if no opposing
reactions come until then.
Christoph
--
----------------------------------------------
Dipl.-Inf., Dipl.-Math. Christoph Hertzberg
Cartesium 0.049
Universität Bremen
Enrique-Schmidt-Straße 5
28359 Bremen
Tel: +49 (421) 218-64252
----------------------------------------------