Phil Steitz wrote:
> Phil Steitz wrote:
>> Thanks, Bernhard for the contribution in MATH-236. I would like to
>> suggest a couple of improvements.
>>
>> First, I think the return type should be List, not Collection, as
>> there is an order to the elements in the returned collection (as
>> stated in the API doc, the order is by increasing argument).
>>
>> Second, since we are also making the method that returns the
>> argument public, I would prefer to name that "getArgument"
>> (preferred) or "getArg" instead of "getPhi".
>>
>> Finally, in the loop that generates the roots, it would be better to
>> compute the pie slice once and then add it each time instead of
>> computing k* 2 * Math.PI/ n for each k > 1.
>>
>> If there are no objections, I will make these changes.
>>
>> Thanks again for the contribution.
>>
>> Phil
> I made the changes above, but as I was updating the tests, I noticed
> another thing. Behavior for complex numbers with NaN and infinite
> parts is, well, "interesting." It is consistent with what we do
> elsewhere in this class to just apply computational formulas and
> document behavior for infinite and NaN; but the current implementation
> is hard to document correctly and the results are a little strange for
> infinite values.
>
> I see three reasonable choices here, and am interested in others'
> opinions. Please select from the following or suggest something else.
>
> 1) Leave as is and just point out that the computational formula will
> behave strangely for infinite values.
>
> 2) return {Complex.NaN} or {Complex.INF} respectively when the number
> has a NaN or infinite part.
>
> 3) return {Complex.NaN} when either part is NaN; but for infinite
> values, compute the argument using getArgument (atan2), generate the
> arguments for the roots from this and select the real/im parts of the
> roots from {0, inf, -inf} to match the argument (this will always be
> possible because atan2 always returns a multiple of pi/4 for infinite
> arguments). For example, the 4th roots of real positive infinity
> would be {inf + 0i, 0 + infi, -inf + 0i, 0 + -infi}
>
> 2) is probably the most defensible mathematically; but 3) is closer to
> the spirit of C99x. Unfortunately, since our implementation of
> multiply is 2)-like, 3) does not guarantee that nth roots actually
> solve the equation r^n = z.
>
> Phil
Sorry, just realized that 3) will not work in general, so choice is 1),
2) or a better idea.
Phil
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org