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

[Benoit Jacob <jacob.benoit.1@xxxxxxxxx>]

2009/10/21 Mark Borgerding <mark@xxxxxxxxxxxxxx>:
> 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 "> >" )

ah, good point, so nevermind this idea.

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

Yes it now seems like the way to go!

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

No no, everything is in namespace Eigen, all I mean is that instead of
doing namespace Eigen {...} in every source file, we use to just do it
once in the big public header, here FFT, and do the #include "src/..."
inside of that.

For example we're soon going to replace namespace Eigen to "ei" and
now it pays off to have centralized that.

>
>
> (to be continued)

Can't wait for the next episode!

And, another TODO point: what about fixed size? Currently, even if we
pass a MatrixBase, the size at compile time isn't used. I don't know
anything about FFT but seem to remember that people did have a use for
a fixed-size FFT.

Benoit