On 9/3/11 5:06 PM, Gilles Sadowski wrote:
>> The problem is that it is not obvious that INF is "the right answer."
> The real problem is *inconsistency*.
> The current code chooses to consider a point at infinity for some situations
> but not for others. It is thus quite understandable that people are
> wondering what is going on with some limiting cases, like using infinities:
> They assume something to understand one result, and the opposite assumption
> must be made to understand another.
>
> You are making the same kind of argument as for "RegulaFalsiSolver" which
> boils down to "The Javadoc says so and so, and the code is good enough for
> me."
> As Sebb (IIRC) pointed out in another discussion, when unobvious results
> are returned, their documentation should mention the rationale that led to
> the chosen behaviour, because this
> 
> If {@code divisor} equals {@link #ZERO}, {@link #NaN} is returned.
> 
> simply looks wrong, unless you are going to maintain that it is perfectly
> reasonable and completely obvious that this test:
> 
> final double rAsComplex = Complex.ONE.divide(Complex.ZERO).getReal();
> final double rAsDouble = 1d / 0d;
> Assert.assertEquals(rAsComplex, rAsDouble, 0);
> 
> should fail.
>
> I maintain that this is an obvious and trivial *bug*. And, however happy the
> current users of the class are, it should be fixed.
>
>>> Maybe that we can _decide_ that any manipulation of a "Complex" instance
>>> containing infinities is mildly meaningless (in the applications where CM
>>> is used) and return NaN for any operation involving them (?). That would
>>> probably simplify several methods (and make them more efficient). Unless I'm
>>> mistaken that would just cost a comment like
>>> "Manipulating instances with infinities will result in undefined results."
>> That is a defensible view. See comments below.
> This at least is *consistent*, saying that
> "We don't handle infinities. Period."
> while the current code is confusing.
>
> Once again, a user is asking for information about a strange behaviour, and
> you insist that he justifies further his request. His issue could be just
> that: Inconsistency.
>
> Now, if the justification is efficiency. Then why not go for it, and just
> not care for infinities, checking for NaN, and tell it explicitly:
> "We don't handle infinities because in most use cases, the efficiency cost
> is not worth it."
>
>>>>> I don't think that this one change can have a discernible performance
impact.
>>>>> It might not be necessary to map all {{Complex}} instances that have
an infinite component to a single object. I pointed it as a convenient justification for fixing
a bug
>>>> Not so clear it is a "bug"  the only way to characterize it as such
>>>> is to model the space as compactified.
>>> As you say yourself below, having an INF is to assume that the "Complex"
>>> class represents elements from C U {"point at infinity"}. At least that's
>>> how I interpret it.
>>> Had there not be a special handling of instances that have an infinity in
>>> their real or imaginary part, I'd say that the NaN result would be normal,
>>> as the output of the computational formulae.
>>> But this would entail to let operations produce their expected results,
>>> which is not the case with "z.multiply(z)", as reported by Arne:
>>> https://issues.apache.org/jira/browse/MATH620?focusedCommentId=13096432&page=com.atlassian.jira.plugin.system.issuetabpanels:commenttabpanel#comment13096432
>>>
>>>> I notice now that multiply sort of behaves this way, and as I said
>>>> on the ticket, we have already defined "INF" so an argument could be
>>>> made that we are partly there already. I would like to understand
>>>> the practical arguments pro and con  and by "practical" I mean
>>>> examples of how the proposed change and any others impact actual
>>>> uses of the class in applications. I have reviewed my own
>>>> applications and for those, there is no immediate impact (other than
>>>> performance hit in division and complex construction),
>>> Did you measure it?
>>> Are you sure that performance is not _improved_ for division? :)
>>>
>>>> but I worry a
>>>> little about losing the ability to represent directed infinities if
>>>> we decide to really aim for "consistency" here.
>>> What are the usecases for those?
>>> When looking for references, I only stumbled on the usefulness of _one_
>>> "point at infinity".
>> When a sequence of iterates diverges in modulus to infinity, it
>> might in some cases be useful to know that the divergence was along
>> a specific vector, like for example the real or imaginary axis. I
>> have  almost  needed this in applications that track orbits of
>> points under iterated functions, but I don't have a specific
>> application use case for this personally.
>>>> I guess we could
>>>> retain directed infinities by adding a directed INF with an argument
>>>> attribute, but that would again add overhead to several operations.
>>> If you have applications that deal with any "directed" infinities, don't you
>>> need polar coordinates too? That seems to be missing in class Complex
>>> (talking about new API...).
>>> Then, would the "argument attribute" be the angle?
>> Yes, the argument would effectively be the angle.
>>>> At this point, I would like to see r1164756 reverted until we have
>>>> agreed on this change.
>>> Well, that is done. And, IMHO, there is an identified bug (or at least
>>> an inconsistency worth removing) there. I would like to see use cases that
>>> justify keeping the inconsistency. A mathematical explanation will do too
>>> ;).
>> The mathematical question is do we view our class as representing
>> the extended complex numbers. If we agree that the answer to that
>> question is yes,
> If you say "no", then my understanding is that the "Complex" class does not
> represent the complex number concept, unless *all* operation that encounter
> infinities result in "undefined behaviour" (i.e. return NaN).
Not necessarily. The Complex field does not include any "point at
infinity." There is a topological space that can be formed from the
Complex numbers by adding a point at infinity and extending
functions to include that point. This is what is sometimes called
the extended complex numbers and a common topology for the space is
represented by the Riemann Sphere. It is not obvious to me that it
is best for us to try to model this (sorry to repeat myself, but
this is the key point). If you really want to be *consistent* and
insist that it is a *bug* to return NaN for division by complex
zero, then you are insisting that we adopt this view. To be
consistent in that view, we need to change equals, all arithmetic
operations, and the exponential and trig functions as well. Our
implementations of those functions are based loosely on C99x, which
does not consistently represent the compactified space.
> As that would arguably be considered not backwardcompatible, that leaves
> only "yes" as the answer.
>
>> then "the right answer" for the one division
>> instance being considered here is to return INF. If we don't, then
>> NaN is a better answer. If we agree yes, then we should also modify
>> equals and there are likely other changes required to other
>> arithmetic operations.
> That would certainly be more consistent with how a "double" represent a real
> number. At which point I'm repeating that you chose to do so in MATH164; so
> why would it be different now?
> The code is inconsistent and, judging how MATH164 was resolved and you want
> to resolve this issue, the CM's decisionmaking process seems to be also.
>
>> IIRC, we tried to find a compromise between efficiency, ease of
>> understanding the javadoc, C99x compliance and mathematical
>> correctness when defining the contracts for the complex arithmetic
>> operations that are documented in the javadoc now. We could fiddle
>> with these contracts endlessly and add lots more ad hoc tests and
>> attributes to try to make them "more correct" or "consistent with X"
>> where X can be defined variously as "the way Doubles work", "IEEE
>> Floating Point specs," "C specs" or your favorite other package.
>> Given that changes carry a cost for all users of the current code, I
>> think the onus is on those who want to make changes to explain in
>> practical terms why the proposed change is worth making. I don't
>> (yet) see this one as worth making. What would be really great
>> would be some input from users who can provide examples why
>> different contracts would be more (or less) useful in applications.
> I'm talking about selfconsistency. That could mean: Avoid contradicting
> behaviours that need confusing documentation.
Sorry, I disagree. I personally don't think it is possible to have
complete "consistency" in the way that you seem to want. I am sure
you could find "inconsistency" in the C99x spec or pretty much any
implementation, even after convoluting the javadoc and killing
performance. If others agree with your view, then we can change the
contracts and implementation. But I really would like to know what
the practical reason is.
>
>>>
>>> Thanks,
>>> Gilles
>>>
>>> P.S. I've just seen that you completely removed changes that I find useful
>>> _independently_ of the decision to change or not the behaviour of
>>> division by zero:
>>> * factoring out multiple tests into separate test methods,
>>> * keeping track of test cases that correspond to an issue, and
>>> * using a flag "isZero" in the "divide" method.
>>> Thus, they were left on purpose, as supposed enhancements similar to
>>> those which several people commit informally from time to time.
>>> Is it customary to pull the rug out from under someone's feet?
>> I asked that the commit be reverted.
> The change of behaviour was reverted; I just forgot to change back the
> Javadoc accordingly.
>
>> I just completed the reversion
>> because I saw that the javadoc contract change had not been
>> reverted. I just used svn merge with revision numbers to do a clean
>> revert.
> Why was a "clean revert" necessary?
> My changes were not a mistake, to be treated as they never occurred.
> Could I respectfully request that you let the concerned people do the
> cleanup instead of forcefully (pardon, "cleanly") erase their work?
Here is how it works. Any committer can 1 a commit. When that
happens, the commit gets reverted. To do that, it is easiest to
just use svn merge with revision numbers. I just completed the
reversion.
>> The added "isZero" attribute is part of the performance
>> hit.
> Really? How much? What tests? Can I emit the possibility that testing a
> "boolean" might be a tiny bit faster than testing equality with "0.0" twice?
The hit is in the constructor, where every complex instance has to
run the code to set the property.
>
>> This, btw, is yet another reason to separate commits.
> OK, I take this as: You would have only "cleanly reverted" the Javadoc
> change, if it would have been separate. I'll thus commit back the rest.
Please do not add back the extra property.
Phil
>
>
> Gilles
>
> 
> To unsubscribe, email: devunsubscribe@commons.apache.org
> For additional commands, email: devhelp@commons.apache.org
>
>

To unsubscribe, email: devunsubscribe@commons.apache.org
For additional commands, email: devhelp@commons.apache.org
