[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=12896845#action_12896845 ] 

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

IMO, there is overlap in that both handle the concept of absolute and relative accuracies (or thresholds, as named in the "...Checker" classes). There shouldn't be two ways for specifying convergence criteria (as I've reported in [MATH-404|https://issues.apache.org/jira/browse/MATH-404]).
What do you think of renaming "ConvergingAlgorithm..." to "IterativeAlgorithm..." and keep there only the "iterationCount" field and related accessors?


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