qSin() giving wrong sin values?

Question

I have the code:

for(int i = 0; i < 314; i++){
    float nextSine = aSin(i/5);
    qDebug() << "i: "<<QString::number(i)<<" sin(i/5) = nextSine: "<<nextSine;
}

And this is a sample of the results I'm getting:

i:  "303"  sin(i/5) = nextSine:  -0.304811
i:  "304"  sin(i/5) = nextSine:  -0.304811
i:  "305"  sin(i/5) = nextSine:  -0.966118
i:  "306"  sin(i/5) = nextSine:  -0.966118

It's right some of the times, but it's wrong other times, for instance when i==303

@KeithThompson: as you could read in the answer, it is a typo. You can also realize that `q` and `a` are close to each other on the keyboard, and it is only typo'd once. There is no valid aSin function either. — László Papp, May 25 '14 at 09:09
@LaszloPapp: Very likely, but the point was to bring it to the attention of the OP. — Keith Thompson, May 25 '14 at 10:09
@KeithThompson: yeah, I agree, thus I also did that already. :) — László Papp, May 25 '14 at 10:15
@JVE999: btw, any reason for using qSin instead of std::sin? — László Papp, May 25 '14 at 14:19
I just wanted to assure maintainability and cross-compilability, so as Qt gets updated, I imagine the code will be easier to update and everything will stay consistent. — JVE999, May 25 '14 at 23:09
@JVE999: std::in has the same maintainability and cross-compatibility. The code will not be easier to update with qSin, nor is it more consistent. Please use std::sin. Also, please do not fix the typo since it partially makes the pointing out useless in the answer(s) causing additional maintenance to the people trying to help. ;) — László Papp, May 26 '14 at 02:26

score 2 · Accepted Answer · answered May 25 '14 at 03:08

2

You probably want to cast to float before running the function:

float nextSine = aSin((float)i/5.0);

answered May 25 '14 at 03:08

laurent

88,262
77
290
428

2

It's enough to just use 5.0 since you want a double. – Retired Ninja May 25 '14 at 03:10
I was going to say I used 5.0 after chris's comment – JVE999 May 25 '14 at 03:11
@JVE999: that is bad. I would personally use type-safe casting in C++ if possible. This is one nice addition compared to C. – László Papp May 26 '14 at 02:25

László Papp · Answer 2 · 2014-05-26T02:23:17.883

1

You have these issues ongoing here:

1) You are trying to divide an integer by 5, which may lose the precision if the integer is not divisible with 5. For instance 313/5=62 and not 62.6 in your case, but that is just one example of those. The solution is to use float explicitly.

2) You have a needless nextSine variable. You could simply eliminate that here.

3) You have a syntax error in your code as you meant **q**Sin, not aSin.

4) Make sure that you inline your function to be as effective as possible.

5) You are trying to use explicit space for printing, but qDebug already manages that for you, so you end up having two instead of the intended one.

So, this is what I would personally write:

for (int i = 0; i < 314; ++i)
     qDebug() << "i:" <<QString::number(i)
              << " sin(i/5) = nextSine:" << qSin(static_cast<float>(i)/5);

or

for (int i = 0; i < 314; ++i)
     qDebug() << "i:" <<QString::number(i)
              << " sin(i/5.0) = nextSine:" << qSin(i/5.0);

or

for (float f = 0; f < 314.0; f+=1.0)
     qDebug() << "i:" <<QString::number(f, 'f')
              << " sin(f/5) = nextSine:" << qSin(f/5);

edited May 26 '14 at 02:23

answered May 25 '14 at 03:46

László Papp

51,870
39
111
135

qsin expects a qreal so it might be better to cast to qreal or use qreal throughout instead of float. – cup May 25 '14 at 05:45
@cup: there is not need for that. Float is just fine for qreal. In fact, the idea is to use less and less qreal in Qt. It should fade away, eventually. It has not that much use anymore. – László Papp May 25 '14 at 06:25
I didn't think about the float for loop. Cool! I made a separate nextSine to make processing easier. Does it hurt processing speed (this is in an animation)? It seems better than attempting to write everything on one line all the time, if it can be written that way. – JVE999 May 25 '14 at 23:12
Last loop is ok in this case, although generally one should be very wary about using floating-point loop counters, since you might get a different number of iterations. For example, using the substitution `for (float f = 0.0f; f < 62.8f; f += 0.2f)` so that `f = i/5.0` would not be equivalent. – Ben Voigt May 25 '14 at 23:16
@BenVoigt: good catch; fixed. For that matter, the printing was mistaken, too. – László Papp May 26 '14 at 02:22
1

@JVE999: There's no performance penalty for storing your intermediate value in a block-local variable. The compiler's going to make a temporary to hold it anyway. – Ben Voigt May 26 '14 at 02:25
@JVE999: that is not the quite the main point, although it is not logical. You use variables in C/C++ when you need to use the same at more than one place unlike here. Readability can be a concern at times, but that is not the use here either. Also, the rest is valid, too, in the answer. – László Papp May 26 '14 at 02:29
@BenVoigt: right, 'i' still remained therein... for the float version, that is, never mind. – László Papp May 26 '14 at 02:31

qSin() giving wrong sin values?

2 Answers2