[ 
https://issues.apache.org/jira/browse/MATH-631?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13080792#comment-13080792
 ] 

Gilles commented on MATH-631:
-----------------------------

I surely hope that your last post is not an answer to mine from 23:46.

I'll try to answer here in case it was in reply to my previous one (23:06).
Of course, the code will not run forever because of the "maxeval" bound.
But it will run for a time that depends on the value of "maxeval" *with no 
added benefit*! From a certain point, the loop is like
{code}
while (true) {
  // Do nothing useful, just count!
  ++count;
  if (count > maxeval) {
    throw new TooManyEvalutationsException(maxeval);
  }
}
{code}

{quote}
from a style perspective, I prefer to explicitly bound loops
{quote}

>From an *OO* style perspective, the reuse of the "Incrementor" is better, and 
>you don't have to rewrite the same "test and throw exception if failed" boiler 
>plate code each time there is such a loop.

{quote}
Trying to detect when a sequence of iterates has gotten "stuck" and is destined 
to hit max iterations without converging is walking down a path that I think is 
unwise for us and users.
{quote}

Why?

{quote}
I see no reason not to stick with the standard impl here
{quote}

A busy idle loop is a compelling reason IMO.

{quote}
Trying to workaround numerical problems in simple algorithms and change 
contracts to include these workarounds is asking for trouble
{quote}

The trouble is there with the current implementation. I'm not criticizing the 
contribution but this issue shows that it should be made more robust.
Also, the documentation about "convergence is guaranteed" can lead to a false 
sense of security.
Moreover, is the "regula falsi" a mathematical algorithm (with a guaranteed 
converge property if computed with infinite precision) or a numerical one, 
which this issue proves that it cannot guarantee convergence? In the former 
case, CM's (numerical) implementation is not strictly "regula falsi" and there 
would be no such thing as respect for an original/standard implementation if we 
can make it more robust.

I've already indicated that the fix does *not* change the contract; it stops 
the busy idle loop as soon as it is detected and reports that it won't do any 
good to increase the number of iterations. That's _obviously_ more robust.

Now, if you were answering to my 23:46 post, I'd be glad to read an explanation 
of why the first paragraph describes expected behaviour.



> "RegulaFalsiSolver" failure
> ---------------------------
>
>                 Key: MATH-631
>                 URL: https://issues.apache.org/jira/browse/MATH-631
>             Project: Commons Math
>          Issue Type: Bug
>            Reporter: Gilles
>             Fix For: 3.0
>
>
> The following unit test:
> {code}
> @Test
> public void testBug() {
>     final UnivariateRealFunction f = new UnivariateRealFunction() {
>             @Override
>             public double value(double x) {
>                 return Math.exp(x) - Math.pow(Math.PI, 3.0);
>             }
>         };
>     UnivariateRealSolver solver = new RegulaFalsiSolver();
>     double root = solver.solve(100, f, 1, 10);
> }
> {code}
> fails with
> {noformat}
> illegal state: maximal count (100) exceeded: evaluations
> {noformat}
> Using "PegasusSolver", the answer is found after 17 evaluations.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Reply via email to