[jira] Commented: (MATH-397) Inconsistencies between "optimization.univariate" and "optimization.general"

["Gilles (JIRA)" <ji...@apache.org>]

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

Gilles commented on MATH-397:
-----------------------------

Made quite some changes in revision 980013.

More work is still required in order to:
* avoid functionality replication (as in the above example)
* simplify some part of the code, as a result of fixing the exception hierarchy; e.g. avoid code like this:
{code}
try {
    functionValue = function.value(opt);
} catch (FunctionEvaluationException e) {
    throw new RuntimeException(e);
}
{code}
(excerpt from method "getFunctionValue" in "AbstractUnivariateRealOptimizer"). [Presently, I couldn't do otherwise since; per the interface, "getFunctionValue" must not throw the checked "FunctionEvaluationException".]


> Inconsistencies between "optimization.univariate" and "optimization.general"
> ----------------------------------------------------------------------------
>
>                 Key: MATH-397
>                 URL: https://issues.apache.org/jira/browse/MATH-397
>             Project: Commons Math
>          Issue Type: Improvement
>    Affects Versions: 2.1
>            Reporter: Gilles
>            Assignee: Gilles
>            Priority: Minor
>             Fix For: 2.2
>
>
> I think that we could make the usage (from the developer's point-of-view) of "optimization.univariate" more similar to what is done in "optimization.general". At first this looked like a small change but then I discovered that "AbstractUnivariateRealOptimizer" competes with "ConvergingAlgorithmImpl" for some functionality, and that everything could be more coherent by enforcing the use of accessors and avoiding "protected" fields.
> Moreover the logic inside  "AbstractUnivariateRealOptimizer" seems convoluted and one change leading to another...
> Currently only "BrentOptimizer" inherits from "AbstractUnivariateRealOptimizer", so I hope that it's OK to revise that class.
> In "ConvergingAlgorithmImpl", I propose to add a method:
> {code}
> protected void incrementIterationsCounter()
>     throws ConvergenceException {
>     if (++iterationCount > maximalIterationCount) {
>         throw new ConvergenceException(new MaxIterationsExceededException(maximalIterationCount));
>     }
> }
> {code}
> This is still not the best since in "BaseAbstractScalarOptimizer", we have
> {code}
> protected void incrementIterationsCounter()
>     throws OptimizationException {
>     if (++iterations > maxIterations) {
>         throw new OptimizationException(new MaxIterationsExceededException(maxIterations));
>     }
> }
> {code}
> (thus: two codes for the same problem, throwing different exceptions).
> Then it seems that there is also a functionality overlap between "ConvergingAlgorithm" and "ConvergenceChecker"...

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.