jiggzson / nerdamer

a symbolic math expression evaluator for javascript
http://www.nerdamer.com
MIT License
517 stars 82 forks source link

asec, acsc and acot related bug fixes. #454

Closed Saikedo closed 5 years ago

Saikedo commented 5 years ago

1)Fixed bug where buildFunction was not working for acsc and asec: The library was trying to look for acsc and asec in Math2 variable and was crashing because Math2 did not contain acsc and asec.

2)Fixed issue where acot was not calculated correctly for values <0: The logic here is this, acot(x) is not always equal to atan(x^1). Here is the actual identity.

acot(x) = atan(x^1) if x > 0. acot(x) = pi + atan(x^1) if x < 0.

I used this logic and implemented the correct routine for calculating the acot.

Note: I did not look into imaginary numbers but as of now acot that have imaginary numbers uses the same logic as before. This might be wrong and I will take a look at that as well.

3)Fixed issue where buildFunction for acot was returning a wrong value: After step 2 was done, there was a need to add acot to Math2 and we have to pretty much follow the same routine that we use for acsc and asec.

jiggzson commented 5 years ago

@Saikedo, I know I'm quite a bit late here but I was really caught up for the last few weeks so thanks for your patience. I'm a little confused as to the motivation to switch how acot is calculated. Using your proposed change, acot(-9) now gives 3.0309354324158977 whereas both Maxima and Wolfram Alpha give -0.1106572211738956. Can you elaborate a little? Thanks for the PR by the way.

Saikedo commented 5 years ago

@jiggzson I had a suspicion that arccot might not be handled correctly on Wolfram Alpha actually for negative numbers and I started doing some research. First of all, both Nerdamer and TI calculators return 3.03 for arccot which makes me even more confused since half of the sources seem to be returning different result from the other half. After that I decided to contact one of my previous professors who is very knowledgable in this type of mathematics and he confirmed my suspicions. Here is a snipped from our conversation.

https://drive.google.com/open?id=1qR8ohhuvXgZZHwp8nfIgAsaGP39Pzal5

Happypig375 commented 5 years ago

🤯

jiggzson commented 5 years ago

And here's the kicker cot(-0.11065722117389) and of course cot(3.0309354324158977). The cyclic nature of trig functions can be very frustrating at times. I quickly read the argument for the proposed definition I'm inclined to agree with your professor. I'm going to read up a little bit to fully convince myself. Very interesting stuff man.

Saikedo commented 5 years ago

And of course I implemented my solution before I talked with my professor and I defined 2 separate cases for x in arccot(x). I am doing a separate calculation in my push when x is negative and when x is positive. I guess we can just skip all that business and just say that arccot(x) = (pi/2) - arctan(x). We do not even need special handling for x = 0 cases.

jiggzson commented 5 years ago

@Happypig375, any thoughts on this one?

Happypig375 commented 5 years ago

Yep, we should follow what the professors say.

Saikedo commented 5 years ago

Oh boy, I really need to learn how to contribute to open source on gitlab :) I messed up the build I think when I was trying to resolve the conflict. I will make another push with this + another fix for BuildFunction. Please ignore this pull request for now

jiggzson commented 5 years ago

Hovo, If you'd prefer you can commit the changes to dev. I'd prefer to move back to that branch. Let me know. Once I get my master branch to pass I'll pull the changes to dev. Shoot me a PR to that branch then. Will that work?

jiggzson commented 5 years ago

I'm guessing this one no longer applies?