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
----------------------------------------------



Mail converted by MHonArc 2.6.19+ http://listengine.tuxfamily.org/