4
\$\begingroup\$

I used C++ metaprogramming to build taylor series for sin, cos, and tan (not really for tan). The whole thing is on my github. Please give any feedback related (but not limited) to style, functionality, etc.

Overall I think the biggest flaw is that the client needs to make their own struct with a static const long double member called value, and pass that struct to the "functions." I would have just passed the double, but C++ doesn't let you use floating-point numbers as template arguments. Using a struct was the only way I could think of to get around that.

Thanks!

#ifndef TRIG_HPP
#define TRIG_HPP

namespace mt
{
    const long double PI = 3.14159265358979323f;

    // FACTORIAL

    template <int N> struct fact
    {
        static const long long value = N * fact<N-1>::value;
    };

    template <> struct fact<1>
    {
        static const long long value = 1L;
    };

    // EXPONENTS

    template <class X, int N> struct power
    {
        static const long double value;
    };

    template <class X, int N> const long double power<X, N>::value = X::value * power<X, N-1>::value;

    template <class X> struct power<X, 0>
    {
        static const long double value;
    };

    template <class X> const long double power<X, 0>::value = 1.0f;

    // SIMPLIFY RADIAN TO INTERVAL [-2π, 2π]

    template <class X> struct SimplifyRad
    {
        static const long double value;
    };

    template <class X> const long double SimplifyRad<X>::value =

        X::value - (2 * PI * (double) (int) (X::value/(2*PI))); // float modulo?

    // COSINE

    template <class X, int N> struct cos
    {
        static const long double value;
    };

    template <class X, int N> const long double cos<X, N>::value = 

        (((N % 2 == 0 ? 1 : -1) * power<SimplifyRad<X>, 2*N>::value) / ((long double) fact<2*N>::value)) + cos<X, N-1>::value;

    template <class X> struct cos<X, 0>
    {
        static const long double value;
    };

    template <class X> const long double cos<X, 0>::value = 1.0f;

    // SINE

    template <class X, int N> struct sin
    {
        static const long double value;
    };

    template <class X, int N> const long double sin<X, N>::value =

        (((N % 2 == 0 ? -1 : 1) * power<SimplifyRad<X>, 2*N-1>::value) / ((long double) fact<2*N-1>::value)) + sin<X, N-1>::value;

    template <class X> struct sin<X, 0>
    {
        static const long double value;
    };

    template <class X> const long double sin<X, 0>::value = 0.0f;

    // TANGENT

    template <class X, int N> struct tan
    {
        static const long double value;
    };

    template <class X, int N> const long double tan<X, N>::value = sin<X, N>::value / cos<X, N>::value;
}

#endif

Example usage:

#include <cstdio>
#include "../include/trig.hpp"

#define DEPTH 8

struct var
{
    static const long double value;
};

const long double var::value = mt::PI / 4;

int main(int argc, char *argv[])
{
    printf("%Lf\n", var::value);
    printf("sin(%Lf) = %Lf\ncos(%Lf) = %Lf\ntan(%Lf) = %Lf\n",
            var::value, mt::sin<var, DEPTH>::value,
            var::value, mt::cos<var, DEPTH>::value,
            var::value, mt::tan<var, DEPTH>::value);
}

Output:

$ g++ main.cpp
$ ./a.exe
0.785398
sin(0.785398) = 0.707107
cos(0.785398) = 0.707107
tan(0.785398) = 1.000000
\$\endgroup\$
1
  • 4
    \$\begingroup\$ I'm not sure how to present it, but I'm afraid parts (if not all) of the code is useless, since C++14 has constexpr. Compilers before C++11 probably will have hard time optimizing out intermediate instantiations, which will lead to huge code bloat. I recommend exploring constexpr world, rather than doing it the way previous generation did. \$\endgroup\$ Commented Aug 6, 2017 at 14:20

1 Answer 1

3
\$\begingroup\$

I see several issues with your code.

Your motivation should be stronger/clearer

While you may have a very good motivation for having this functionality, it is not apparent from your post hear nor from your GitHub repo. At compile time there are typically very few values I'd need the sine or cosine of; is it really clearer to write

printf("%f", apply_my_complicated_TMP_construct(PI/4, 8) )

than

auto sine_of_quarter_pi = 0,707106781187f;
printf("%f", sine_of_quarter_pi);

? I don't know that it is. Plus, the second way the reader doesn't need to think about iterations, the taylor series etc. It will also reduce compilation time.

Now, you could say "Ah, but I have a lot of these values to apply sin() or cos()" - but honestly, I doubt it. If that's the case you should just do that offline and independently of developing your C++ code.

In your repository, you list the following advantages:

  • All the work is done a compile-time.
  • The compile-time efficiency is actually going to be better than run-time efficiency compared to "normal,"

But, again, there isn't really any work; and there's no "efficiency" issue if you just use the value you need. Even if you had to calculate a sine or cosine a couple dozen times - that's really negligible.

Don't reinvent the wheel I

There's an M_PI constant available in POSIX-compliant system's <math.h> header (and thus, effectively, in most <cmath> headers, including MSVC's with a bit of work, see here). In case you want to be more portable, you might do:

#ifndef M_PI
    #define M_PI 3.14159265358979323846
#endif

... but, in fact, these days you can even get a better accuracy constant, M_PIl, on some systems. So you would have actually gained from not reinventing the wheel.

... and there's more. Remember my sqrt(2)/2 from the example above? That's often predefined too. Have a look at your system's <math.h>.

Don't reinvent the wheel II

Someone has already provided what looks like a more robust implementation of the same functionality - here on the site:

constexpr Sin Function C++ 14

Now, granted, I haven't scrutinized that code. Maybe it's buggy or has other problems. But I'll bet you 100 reputation you haven't read that implementation before writing yours... although you could have, if you'd have looked. I didn't know about it before I was writing this review.

Drop most template recursion in favor of C++14 constexpr functions

You use recursive template instantiation when you could just put a for loop in your function - and sometimes not even need a template:

constexpr long long factorial(int n)
{
    return (n <= 1) ? 1L : (n * factorial(n - 1));
}

(taken from here). Also, when you can't just "go constexpr", you can still iterate over multiple template arguments with for_each_argument and so on.

\$\endgroup\$
2
  • \$\begingroup\$ While it's good advice to use it where it's available, M_PI is a POSIX rather than a C++ standard. \$\endgroup\$
    – Edward
    Commented Aug 6, 2017 at 21:14
  • \$\begingroup\$ @Edward: Right you are. \$\endgroup\$
    – einpoklum
    Commented Aug 6, 2017 at 21:24

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.