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

Gilles commented on MATH-404:
-----------------------------

Sorry I hadn't followed that other report.

{quote}
It was decided to let both convergence methods available. 
{quote}

Switching between two convergence checking procedures, based on whether a field 
is {{null}} or not, is at best a temporary workaround, but it is not a good 
solution.

As explained above, from an OOP point-of-view, it is surprising that a class 
completely circumvents its base class interface.
At least one of the following is wrong:
* {{LevenbergMarquardtOptimizer}} inherits from 
{{AbstractLeastSquaresOptimizer}}
* {{LevenbergMarquardtOptimizer}} has a second interface for convergence 
checking
*  {{AbstractLeastSquaresOptimizer}} defines the interface for  convergence 
checking

{quote}
[...] does not fit with the general scheme.
{quote}

Then maybe the scheme needs to be reviewed so that it is general enough to fit.
Allow me to remind what you said: convergence checking is independent from the 
optimization algorithm.
But then, in the LM implementation, this doesn't hold...

If it is really impossible to fit LM within the hierarchy it currently belongs 
to, then it should not belong to it, since one cannot leverage the advantages 
of "interface programming" anyways.


> Confusing interface for "LevenbergMarquardtOptimizer"
> -----------------------------------------------------
>
>                 Key: MATH-404
>                 URL: https://issues.apache.org/jira/browse/MATH-404
>             Project: Commons Math
>          Issue Type: Bug
>    Affects Versions: 2.1
>            Reporter: Gilles
>             Fix For: 2.2
>
>
> {{LevenbergMarquardtOptimizer}} inherits from 
> {{AbstractLeastSquaresOptimizer}} which in turn implements 
> {{DifferentiableMultivariateVectorialOptimizer}}. That interface mandates 
> methods for setting and getting a {{VectorialConvergenceChecker}}.
> In v2.1, however, that checker is never used! The convergence check is 
> performed using parameters specific to the Levenberg-Marquardt algorithm. 
> Such circumvention of the superclass interface is confusing and leads to 
> totally unexpected behaviour (such as changing the values of the thresholds 
> of the {{VectorialConvergenceChecker}} being ineffective).
> In the development version, the default constructor of 
> {{LevenbergMarquardtOptimizer}} sets the the {{VectorialConvergenceChecker}} 
> field to "null" and when such is the case, the behaviour is as in v2.1. 
> Although it is documented, this is still confusing since it is impossible to 
> use {{LevenbergMarquardtOptimizer}} through its 
> {{DifferentiableMultivariateVectorialOptimizer}} interface: When using the 
> {{VectorialConvergenceChecker}}, one does not know what parameters to use in 
> order to reproduce the results obtained with the LM-specific convergence 
> check (i.e. how to reproduce the result from v2.1).
> Unless I'm missing something, I think that there should be an LM-specific 
> implementation of {{VectorialConvergenceChecker}} that, when given the usual 
> relative and absolute thresholds, can perform a check that will give the same 
> result as the currently specific check (when the "checker" field is "null").

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

Reply via email to