Re: [eigen] Eigen::FFT and Eigen::Complex in repo

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


Benoit Jacob wrote:
FFT line 48:

template <typename _Scalar,
         typename _Traits=DEFAULT_FFT_IMPL<_Scalar>
         >

First, why call it Traits when Impl seems to be what it really is?
Done.


Also, here you're passing _Scalar twice, and are using a macro. You
can improve on both fronts in 2 ways:
* either you let _Scalar be deduced from _Impl

My thinking behind the dual argument was to allow quick and easy declaration from the user standpoint.
e.g.
   FFT<float>
where the implementation that gets used is the best available at compile time. Rather than
   FFT< ei_kissfft_impl<float> >
which seems verbose and inflexible
(also notice the error prone "> >" )

I agree that the use of preprocessor macros to define the default is less than ideal. Perhaps it should not default to the best available, but rather to whichever library is requested via preprocessor definitions.


* or you can keep the _Scalar parameter and let _Impl be a template
template parameter:

template <typename _Scalar, template<typename t> class _Impl>
class FFT
{
...
};

I have not done this before. I will give it a try.


ei_fftw_impl.h line 25:

the using namespace Eigen is best centralized in FFT.
Can you elaborate? Do you mean the ei_* classess should be at the global scope?


(to be continued)





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