commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gilles Sadowski <gil...@harfang.homelinux.org>
Subject Re: [math] Complex division
Date Sun, 04 Sep 2011 00:06:41 GMT
> 
> 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/MATH-620?focusedCommentId=13096432&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13096432
> >
> >> 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 use-cases 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).
As that would arguably be considered not backward-compatible, 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 MATH-164; so
why would it be different now?
The code is inconsistent and, judging how MATH-164 was resolved and you want
to resolve this issue, the CM's decision-making 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 self-consistency. That could mean: Avoid contradicting
behaviours that need confusing documentation.

> 
> >
> >
> > 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?

> 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?

> 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.


Gilles

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message