Modified the handling of zero and infinity to map to the Riemann sphere. #17
Conversation
…se() function return the value of P. The parse() function is now only called in the constructor Complex(a, b) and all other calls to parse() have been replaced with 'var z = new Complex(a, b)'. The variable z is used to avoid confusion with the old P global variable. NOTE: I left the test file pointing to the complex.js file instead of the complex.min.js file so that should be changed prior to integration.
…ity at the north pole and a single zero at the south pole. All operations are consistent with the definitions documented here: https://en.wikipedia.org/wiki/Riemann_sphere#Arithmetic_operations
| @@ -1238,6 +1317,8 @@ | |||
| Complex['I'] = new Complex(0, 1); | |||
| Complex['PI'] = new Complex(Math.PI, 0); | |||
| Complex['E'] = new Complex(Math.E, 0); | |||
| Complex['INFINITY'] = new Complex(Infinity, Infinity); | |||
There was a problem hiding this comment.
I would prefer calling this ComplexInfinity or simliar to make it clear that this is different from the real infinity that people are used to in JavaScript
There was a problem hiding this comment.
I suppose the same could be said for the NAN. But since they are all CAPS and prefixed by the type (Complex.INFINITY and Complex.NAN) I am not sure the clarification is necessary. Doesn't matter to me though...
There was a problem hiding this comment.
Even if ComplexInfinity is more unambiguous, I like Complex.INFINITY and Complex.NAN better, since you would normally use it exactly as "Complex.INFINITY" and so it states already what ComplexInfinity wants to express.
There was a problem hiding this comment.
I guess the question is how strongly do you feel about having the uppercase names. People are used to writing NaN rather than NAN so the upper case version may be confusing.
complex.js
Outdated
| @@ -130,6 +128,8 @@ | |||
|
|
|||
| var parse = function(a, b) { | |||
|
|
|||
| var P = {'re': 0, 'im': 0}; | |||
|
|
|||
There was a problem hiding this comment.
Should the P variable be changed to lowercase now that is is no longer a global? Perhaps change it to z to reflect its purpose?
| @@ -1238,6 +1317,8 @@ | |||
| Complex['I'] = new Complex(0, 1); | |||
| Complex['PI'] = new Complex(Math.PI, 0); | |||
| Complex['E'] = new Complex(Math.E, 0); | |||
| Complex['INFINITY'] = new Complex(Infinity, Infinity); | |||
There was a problem hiding this comment.
Even if ComplexInfinity is more unambiguous, I like Complex.INFINITY and Complex.NAN better, since you would normally use it exactly as "Complex.INFINITY" and so it states already what ComplexInfinity wants to express.
| @@ -385,7 +447,7 @@ var tests = [{ | |||
| set: "0-0i", | |||
| fn: "pow", | |||
| param: 0, | |||
| expect: "0" | |||
| expect: "1" | |||
There was a problem hiding this comment.
What is the rationale behind "0^0"=1?
There was a problem hiding this comment.
as x -> 0, x^x -> 1 could be the reason
There was a problem hiding this comment.
Yep, that makes sense. But typically 0^0 is undefined. So 0 wasn't the best option either, but the question is if 0^0 should be defined as 1. I mean, Math.pow(0,0)=1 having it for complex numbers equal to one would make it consistent.
There was a problem hiding this comment.
It's a very good point. It seems like it should be either Complex.ONE or Complex.NAN. The IEEE floating point standard defines it both ways according to wikipedia: https://en.wikipedia.org/wiki/Zero_to_the_power_of_zero#IEEE_floating-point_standard
There was a problem hiding this comment.
I would say let's go with Complex.ONE with a small note in the readme.
|
Besides my small annotations, this looks pretty good to me. @harrysarson What do you think about it in contrast to your previous work towards infinity support? |
…is no longer a global variable. Updated the README file to include the new complex constants.
|
What do we do with |
|
@harrysarson that would be great, thanks! Trig functions under Infinity is a whole new topic. Do you know a resource of what the behavior should be, besides |
With regards to #5 I took a shot at the option of mapping the complex plane to the Riemann sphere as suggested by @balagge and referencing the wikipedia definition: https://en.wikipedia.org/wiki/Riemann_sphere
The code assumes that there is only one infinity at the north pole and only one zero at the south pole. I added Complex.INFINITY = new Complex(Infinity, Infinity) as the singular infinity pole (the Complex.ZERO already exists. The way to think about this is that each pole collapses to a scaler value rather than a complex value. The operations are well defined mathematically and I added test cases for all of the important ones involving zero and infinity documented at the wikipedia site: https://en.wikipedia.org/wiki/Riemann_sphere#Arithmetic_operations . I also fixed the test cases that were incorrect according to the Riemann mapping.
Take a look at it and see if it still meets the needs of most users. The size of the code did not grow too much and I was able to add quite a few "short circuits" for the Riemann cases to help speed up processing. Thoughts?