Bug 190098 - kcalc error in trigonometric functions
Summary: kcalc error in trigonometric functions
Status: RESOLVED FIXED
Alias: None
Product: kcalc
Classification: Applications
Component: general (show other bugs)
Version: unspecified
Platform: openSUSE Linux
: NOR normal
Target Milestone: ---
Assignee: Klaus Niederkrüger
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-04-19 20:06 UTC by Harald Nikolisin
Modified: 2009-05-21 00:19 UTC (History)
4 users (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Harald Nikolisin 2009-04-19 20:06:49 UTC
Version:            (using KDE 4.2.2)
OS:                Linux
Installed from:    SuSE RPMs

sin/cos/tan are broken!!

kcal (4.2.2): DEC + DEG are shown in the output line
cos(angle) is always 1
sin(angle) is always 0 execept of 90deg, then it's 0
Comment 1 Burkhard Lück 2009-04-22 17:24:25 UTC
I can comfirm this bug for kcalc in kde 4.2.2 and trunk.

In kde 4.1 sin/cos/tan work properly with the latest kcalc branch/4.1 compiled from sources:
$ kcalc --version
Qt: 4.4.0
KDE: 4.1.2 (KDE 4.1.2)
KCalc: 2.4

I compared the sources of kcalc from branch 4.2 and branch 4.1, but could not find any changes, that could be the reason for this bug.

So I copied the kcalc folder from my 4.1 system to my 4.2 system and build it from sources:
$ kcalc --version
Qt: 4.5.0
KDE: 4.2.2 (KDE 4.2.2)
KCalc: 2.4

And now in kcalc (build from the sources of branch 4.1) sin/cos/tan are broken!

So the reason is either the change from Qt 4.4.0 to Qt 4.5.0 or changes in kdelibs from 4.1.2 to 4.2.2
Comment 2 Médéric Boquien 2009-05-04 06:17:14 UTC
Hi!

I have investigated the bug a bug and it is really weird. It appears that the float KNumber constants are set the 0 (with the correct NumType FloatType) while the non float ones have their correct value, as if the initalisation did not work properly for floats. Now, if i create a float KNumber elsewhere in the code, this number is correctly defined, so basically two calls to create a float KNumber do not produce the same result.
Comment 3 alex 2009-05-19 16:38:45 UTC
It seems to be depending on deg/rad/grad-settings. In rad-mode, the results are as expected, but deg and grad mode fail.

E.g. calculate sin(45°deg), sin(50°grad) and sin(pi/4) in rad mode. The rad-mode shows the correct result of 1/sqrt2, and also asin(1/sqrt2) shows correctly pi/4. both grad and deg mode fail, i.e. sin(x!=n90°)  with even n show 0. The asin function is broken as well and show "inf" as result anything else than 0 and +/- 1.

After digging through the source and adding some debug statements, I found out that KNumber::Pi contains 0 (as well as e and all other static constants defined in KNumber.cpp). Replacing the 
KNumber const KNumber::Pi ("3.141592653589793238462643383279502884197169...");
by
KNumber const KNumber::Pi (3.141592653589793238462643383279502884197169);

in knumber.cpp, line 40 solved the problem (Note the quotation marks!!), now all trigonometric functions seem to work properly.

It seems as if the problem is caused by an implicit conversion from const char* to QString when calling the constructor of KNumber.
Comment 4 Médéric Boquien 2009-05-19 16:46:47 UTC
This is basically what i said in the previous comment. However using KNumber const KNumber::Pi (3.141592653589793238462643383279502884197169); is _not_ something you want to do because then you lose the precision on pi which falls to a double precision. The good solution is to fix the constructor. Now if you find what is wrong with is you are a winner. The non static version works fine. After, if no fix is found by 4.3, that workaround will have too be applied.
Comment 5 alex 2009-05-19 17:42:21 UTC
In principle you're right with the precision, but in the specific case the effective precision is that from double, since the number is converted to double in SinDeg & co.

Nevertheless, I digged somewhat deeper into the code, and I think I've found the real problem: in _knumfloat::_knumfloat(QString const & num), the input is assigned using mpf_set_str. Unfortunately, the return-code of this function is ignored, and I've printed it -- voilá, it *failed*. It couldn't assign "3.141..." to _mpf. the I've tried with "3141" explicitly which worked, which made me think of a locale problem. Tried "3,141...", and -- surprise -- it worked. Then started kcalc with $LANG=en_US instead of de_AT, and it works. Obviously, in the german locale I used gmp interprets "," as comma and not "." as in english.

conclusion: a nice side effect of localization causing things to fail in an entirely unexpected way...
Comment 6 Médéric Boquien 2009-05-19 17:55:53 UTC
Oh, excellent! I will look into that later today. I think the fix should be quite easy now. Thanks for investigating the cause of the problem.
Comment 7 Médéric Boquien 2009-05-20 06:35:56 UTC
SVN commit 970340 by mboquien:

The problem was that static initialisation occurs before entering main() and therefore before the C locale was forced in kcalc.cpp. This in turn generated a problem for constants defined as a string 
if the user locale defined the decimal separator not as a ".". Thanks to alex for the source of the problem the problem, Hub for the explanation and Maelcum who pointed to the solution, now Pi is 
initialized only when the class is instantiated the first time, which solves the problem. It just has to be called as a function rather than a constant now. If someone familiar with K_GLOBAL_STATIC_WITH_ARGS 
could check what i did i would be grateful, i am a bit reluctant to backport it to 4.2 without an OK first.

BUG:190098
CCMAIL:ahartmetz@gmail.com
CCMAIL:lemma@confuego.org


 M  +2 -2      kcalc_core.cpp  
 M  +4 -4      kcalc_core.h  
 M  +21 -7     knumber/knumber.cpp  
 M  +2 -2      knumber/knumber.h  


WebSVN link: http://websvn.kde.org/?view=rev&revision=970340
Comment 8 Médéric Boquien 2009-05-21 00:19:11 UTC
SVN commit 970831 by mboquien:

Backport r970340.

CCBUG:190098


 M  +2 -2      kcalc_core.cpp  
 M  +4 -4      kcalc_core.h  
 M  +19 -6     knumber/knumber.cpp  
 M  +2 -2      knumber/knumber.h  


WebSVN link: http://websvn.kde.org/?view=rev&revision=970831