Skip to content

Parse and mul functions #6

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Jul 9, 2017
Merged

Conversation

harrysarson
Copy link
Contributor

A possible solution to multiplication of infinite numbers and extensions to the parse function to support infinity better.

See issue #5

@harrysarson
Copy link
Contributor Author

ok, sorry I had exams so did't have time to properly look at this.

This only test that now fails is i * Infinity which "should" be NaN but is now imaginary infinity.

Would be happy to extend the test suit to properly test infinite values if you are happy with the new behaviour.

@infusion
Copy link
Collaborator

I like your pull request and I think I'm happy with the change. The question I'm still struggling with is how we should represent complex infinity. I think INF/INF isn't the best implementation for that. What do you think?

From my perspective, it's totally fine to make i*Infinity NaN. One last question about your code:

How does

          P['im'] = a['arg'] === 0
             ? 0
             : a['abs'] * Math.sin(a['arg']);

make any difference? Am I missing something?

@harrysarson
Copy link
Contributor Author

INF/INF

  • I represented ComplexInfinity this way as it did not require adding an extra property to complex objects.
  • I also tried giving each object a Boolean flag and even a completely distinct type to represent infinity but both ended up more complicated.
  • The mul function is the uglyist function by far - every other function that I looked at requires only minor changes to support infinities in this way.
  • toString could do some formatting so that the user wouldn't need to know that ComplexInfinity is represented as INF/INF.

i*Infinity

  • If 1 * Infinity is supported by complex.js then I think it follows that i * Infinity should also be supported.
  • Wolfram defines ComplexInfinity to have indeterminate or unkwon argument and so i * Infinity cannot be ComplexInfinity so I thought it made sense to have it as an imaginary infinity.

Parsing

 P['im'] = a['arg'] === 0
             ? 0
             : a['abs'] * Math.sin(a['arg']);
  • Is there to convert { abs: Infinity, are: 0} to { re: Infinty, im: 0 }.
  • With out this conditional: abs * sin(arg) === Infinity * 0 === NaN.

@infusion
Copy link
Collaborator

INF/INF

  • The Question is, if INF/INF is the best way to do this. I'm not completely sure yet, but maybe 0/INF is a better option (from a mathematical standpoint). From a technical point of view, it would be nice if we could reduce the amount of code needed to handle these cases. But for now it's ok.
  • I'm not in favor of a boolean flag for infinity.
  • I think many functions handle the Infinity case correctly already. Question is, if we should fix all the trig functions as well - which would require loads of code addition
  • Yep, toString definitely should handle the infinity case to make it pretty.

i*Infinity

Makes sense

Parsing

Makes sense. Could you please add a small comment on the reasoning to the code?

@harrysarson
Copy link
Contributor Author

At the moment 0/INF represents imaginary infinity.
Currently a complex object can either be:

  1. Finite.
  2. Plus or minus {re: Infinity, im: 0}.
  3. Plus or minus {re: 0, im: Infinity}.
  4. {re: Infinity, im: Infinity} aka ComplexInfinity .

Values like {re: 5, im: Infinity} get coverted to ComplexInfinity.

As far as I can tell, most functions do handle it correctly.

Trig is tricky. sin(Infinity) oscillates but sin(i * Infinity) would tend to (real) Infinity.
Maybe just return ComplexNaN.

@harrysarson
Copy link
Contributor Author

I have entirely re-written the mul function.

Now:

  • First the function checks if the result will be NaN.
  • Otherwise carries out the multiplication coercing all NaN values to zero.
  • Constructing a new Complex object will then convert value like 5 + ∞i to ComplexNaN.

@infusion
Copy link
Collaborator

infusion commented Jul 9, 2017

This is pretty nice! Thanks for the heavy work on this subject. I really like the current version :)

@infusion infusion merged commit c93ea31 into rawify:infinity Jul 9, 2017
@harrysarson harrysarson deleted the infinity branch July 9, 2017 12:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants