[ 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