0

I'm trying to calculate the sin with the Taylor series but for some reason it doesn't work and it's returning big numbers (bigger than 1). I saw some examples on google but I don't see where I did a mistake. here's the code:

#include <stdio.h>
#include <math.h>

int factorial(int n) {
    int i, f=1;
    for (i=1; i<=n; i++) {
        f= f*i;
        }
    return (f);
}


int main() {
    int x, i, j=0;
    float s=0;
    
    printf("write a number to calculate sin(x)\n");
    scanf("%d",&x);
    
    for(i=1;i<=x ;i=i+2) {
        if(j%2==0){
            s -= pow(x, i)/factorial(i);
            }
        else{
            s += pow(x, i)/factorial(i);
            }
        j++;
        }
    printf("sin(%d)=%f \n", x, s);
}

here's an example output: sin(6)=-34.799999

but in a calculator: sin(6)=-0.27941549819

Mhmd Admn
  • 357
  • 3
  • 10
  • Edit the question to show an example of input for which the program produces incorrect output, show the output the program gives, and show the output you desire instead. When asking about bugs in programs, always include a [mre]. – Eric Postpischil Feb 08 '21 at 19:23
  • Why are you using ints? In particular, for x? – stark Feb 08 '21 at 19:26
  • @EricPostpischil I edited the question – Mhmd Admn Feb 08 '21 at 19:27
  • Do you expect this to calculate the sine of `x` in degrees? It will calculate the sine of `x` in radians. `x` should be a floating-point number. Change the declaration of `x` to `float` or `double` and change the conversion specifications in `scanf` and `printf` to match. You may also want `s` to be a `double` instead of a `float`. – Eric Postpischil Feb 08 '21 at 19:27
  • 1
    `x` is the wrong bound to use for `i` in `for(i=1;i<=x ;i=i+2)`. For the moment, change it to 20 or so. Later, you can think about making the loop iterate until the terms being added are too small to affect the result. It will take more terms for larger `x`. – Eric Postpischil Feb 08 '21 at 19:28
  • Note that 12! is as far as you can go without overflow in the `factorial()` function. – Weather Vane Feb 08 '21 at 19:29
  • @EricPostpischil I did all that, and here's the output now sin(6.000000)=5616632.500000 – Mhmd Admn Feb 08 '21 at 19:29
  • @WeatherVane why? is there a mistake? – Mhmd Admn Feb 08 '21 at 19:30
  • 13! is 6227020800 but `INT_MAX` is 2147483647. – Weather Vane Feb 08 '21 at 19:31
  • Do you know the idea of an integer division? It means that `9/4 = 2` instead of 2.25. If you want the real answer (the floating point one), you need to make sure you are not dividing integers, e.g. like this: `(double)9 / 4`, that will give you 2.25. – Dominique Feb 08 '21 at 19:31
  • @Dominique so put everything in float? – Mhmd Admn Feb 08 '21 at 19:32
  • Also, use `double` instead of `float`, it's more precise. – Dominique Feb 08 '21 at 19:32
  • 1
    The Taylor series is “better” for small `x`: It converges more quickly and requires fewer terms to be accurate. When `x` is 6, the numerator of each term increases by 36 (6^2), so the factorials are not decreasing the terms by as much for a while. You will overflow the `int` type used for the factorial before you get good results. You can improve that some by changing `int` to use `double`. But get your code working for small values first, then think about changing it for large values. – Eric Postpischil Feb 08 '21 at 19:32
  • @EricPostpischil how to make it work for small values? – Mhmd Admn Feb 08 '21 at 19:33
  • Make the changes I have described (but keep the loop limit to 12 until you change `factorial` to use `double`). – Eric Postpischil Feb 08 '21 at 19:33
  • @EricPostpischil I did – Mhmd Admn Feb 08 '21 at 19:34
  • Okay, so test your code with small values, like .5 and .7. Does it get the right results? – Eric Postpischil Feb 08 '21 at 19:34
  • Also swap `s -=` and `s +=` or change `j%2==0` to `j%2!=0`; you have the signs the wrong way around. – Eric Postpischil Feb 08 '21 at 19:34
  • @EricPostpischil after swapping the += and -= it's working but only for the small numbers like .5 and .6 – Mhmd Admn Feb 08 '21 at 19:36
  • Change `factorial` to use `double` in its return value and in its `f`. – Eric Postpischil Feb 08 '21 at 19:37
  • @EricPostpischil I don't understand, after using double it worked for sin(6) but sin(77.000000)=-5415048037552619520.000000 – Mhmd Admn Feb 08 '21 at 19:40
  • @EricPostpischil it's working only for small numbers like 5 6. do you think I should convert to radians? thanks btw for helping me – Mhmd Admn Feb 08 '21 at 19:43
  • Research `remquo()` to help bring the angle into a range [0...pi/4] and report the octant. – chux - Reinstate Monica Feb 08 '21 at 19:47
  • @chux-ReinstateMonica thanks I will take a look – Mhmd Admn Feb 08 '21 at 20:00
  • @MhmdAdmn [remquo](https://stackoverflow.com/a/31525208/2410359) sample usage. Consider making your own `cos()` and `sin()`, not just one. – chux - Reinstate Monica Feb 08 '21 at 20:06
  • Hint: take a hand calculator and try to use it to estimate `sin(77)` using the Taylor series. You'll see pretty quickly what the problem is. – Nate Eldredge Feb 08 '21 at 21:25
  • usually, it's not a good idea to optimise your code prematurely. However, Taylor series are the exception to that rule. Even if you believe the pow function is more accurate than repeatedly multiplying with x^2, at least calculate the factorials by multplying the previous result with (n+1)*(n+2) – Paul Janssens Feb 09 '21 at 09:04
  • I don't think this series is defined outside the range +/- pi. – stark Feb 09 '21 at 16:43

0 Answers0