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