Re: [eigen] Accepting different matrix type in the decompositions

[ Thread Index | Date Index | More lists.tuxfamily.org/eigen Archives ]


2010/7/20 Benoit Jacob <jacob.benoit.1@xxxxxxxxx>:
> 2010/7/20 Gael Guennebaud <gael.guennebaud@xxxxxxxxx>:
>> Hi,
>>
>> this makes sense to me.
>>
>> Note that we have to make sure the actual decomposition is not
>> performed by the templated compute() method, but by an internal
>> function (to reduce binary code duplication...). This is already the
>> case for a few decs (typically the ones working per block), but not
>> for all. Anyway, this is a simple change.
>
> I agree with Adrien's suggestion and with your remark :-)
>
> Benoit

I also notice something very encouraging: in most or all of the
suggestions we've recently received, even when they showed that
internal changes were needed, the good thing is that they were
internal and didn't require to break existing interface. A good sign
that we're getting closer to a release!

Benoit


>
>>
>> gael
>>
>> On Tue, Jul 20, 2010 at 9:52 AM, ESCANDE Adrien 222264
>> <adrien.escande@xxxxxx> wrote:
>>>
>>> Hello again,
>>>
>>> the different decompositions (QR, LU...) I looked at are all templated by a type MatrixType and have, among others, a method compute(const MatrixType& matrix) with a line m_matrix = matrix, and a constructor
>>> Decomposition(const MatrixType& matrix) calling compute.
>>> I think it wouldn't hurt to template these two methods by a type InputMatrixName:
>>> template<typename MatrixType>
>>> template<typename InputMatrixType>
>>> Decomposition<MatrixType>::Decomposition(const InputMatrixType& matrix)
>>>
>>> and
>>>
>>> template<typename MatrixType>
>>> template<typename InputMatrixType>
>>> Decomposition<MatrixType>::compute(const InputMatrixType& matrix)
>>>
>>> this way, when InputMatrixType is different from MatrixType, there is no need to create a temporary object for the conversion, and this conversion is only made at the line m_matrix  = matrix.
>>>
>>> In the usual case where the user use the .decomposition() method of MatrixBase<Derived> (for example matrix.colPivHouseholderQR()), we have MatrixType = Derived::PlainObject and InputMatrixBase = Derived, and there's no need to call eval() within decomposition.
>>>
>>> And this simplify the writing in other cases. My use case (a complete orthogonal decomposition):
>>> ColPivHouseholderQR<MatrixXd> LQ(M.transpose());
>>> HouseholderQR<MatrixXd> qr(LQ.matrixQR().topLeftCorner(LQ.rank(), LQ.cols()).triangularView<Upper>().transpose());
>>> I don't even want to imagine the MatrixType I should have wrote for the HouseholderQR...
>>> By adding the InputMatrixType template, I avoid there two temporaries.
>>>
>>> I wanted to add a static assert to check for the compatibility of MatrixType and InputMatrixType, but ei_is_same_type<MatrixType, InputMatrixType::PlainObject>::ret seems to be a too strict condition. I just test the same scalar type for now.
>>>
>>> Adrien
>>>
>>>
>>>
>>
>>
>>
>



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