You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "Gilles (JIRA)" <ji...@apache.org> on 2010/07/27 17:19:16 UTC
[jira] Created: (MATH-397) Inconsistencies between
"optimization.univariate" and "optimization.general"
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.
[jira] Updated: (MATH-397) Inconsistencies between
"optimization.univariate" and "optimization.general"
Posted by "Gilles (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-397?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Gilles updated MATH-397:
------------------------
Fix Version/s: 3.0
(was: 2.2)
Incompatible changes: Postponing commit to after 2.2 branch creation.
> 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: 3.0
>
>
> 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.
[jira] Commented: (MATH-397) Inconsistencies between
"optimization.univariate" and "optimization.general"
Posted by "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.
[jira] Issue Comment Edited: (MATH-397) Inconsistencies between
"optimization.univariate" and "optimization.general"
Posted by "Gilles (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-397?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12897242#action_12897242 ]
Gilles edited comment on MATH-397 at 8/12/10 8:09 AM:
------------------------------------------------------
I'm about to create an {{IterativeAlgorithm}} interface and its corresponding "...Impl" class.
While at that I'd like to have the {{incrementIterationCount()}} method of that class throw an instance of a new {{TooManyIterationsException}} class that would be an _unchecked_ exception. I think that it would be fine that such an exception inherits from the standard {{IllegalStateException}} (passing through a yet-to-be-defined, localizable, {{MathIllegalStateException}}, of course...).
Do you agree?
> 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.
[jira] Commented: (MATH-397) Inconsistencies between
"optimization.univariate" and "optimization.general"
Posted by "Luc Maisonobe (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-397?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12893160#action_12893160 ]
Luc Maisonobe commented on MATH-397:
------------------------------------
About ConvergingAlgorithm and ConvergenceChecker, I don't think there is a real overlap.
The first one is merely an interface for a complete algorithm whereas the second one is an interface to be implemented by user (if our own implementations do not suit him) for one specific problem. It is the way that was chosen to have general search algorithms that can be used with problem-specific convergence conditions.
If however we consider there is an overlap and something difficult to understand, then ConvergingAlgorithm should be the interface that could be removed and ConvergenceChecker is the important interface that must be kept.
> 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.
[jira] Commented: (MATH-397) Inconsistencies between
"optimization.univariate" and "optimization.general"
Posted by "Gilles (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-397?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12893155#action_12893155 ]
Gilles commented on MATH-397:
-----------------------------
In "ConvergingAlgorithmImpl", I've modified {{incrementIterationsCounter}}:
{code}
protected void incrementIterationsCounter()
throws MaxIterationsExceededException {
if (++iterationCount > maximalIterationCount) {
throw new MaxIterationsExceededException(maximalIterationCount);
}
}
{code}
The use of a wrapping {{ConvergenceException}} is not sensible as, at the level of that class, the concept is really "MaxIterationsExceeded" (and the class manages fields referring to "iterations").
> 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.
[jira] Commented: (MATH-397) Inconsistencies between
"optimization.univariate" and "optimization.general"
Posted by "Gilles (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-397?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12900233#action_12900233 ]
Gilles commented on MATH-397:
-----------------------------
Another source of confusion: in package "optimization", the terms "Scalar" and "Real" are used to construct various class names (e.g. "MultivariateRealOptimizer" and "RealPointValuePair"). Since there is a "DifferentiableMultivariateVectorialOptimizer" counterpart, I think that the correct term is "Scalar"; hence we should rename all the classes that use "Real" in that sense.
For the classes in package "optimization", it's just one more step towards in the current rationalizing of the code; however that could extend beyond it since the problem probably stems from the naming of the "function" interfaces (in package "analysis"):
{{MultivariateRealFunction}} vs {{MultivariateVectorialFunction}}. This name change will trickle down to many classes in all CM.
I'm for including this change in 3.0, together with going from a checked "FunctionEvaluationException" to an unchecked one. We trail this checked exception (mandatory "throws" clauses) everywhere without ever using the feature inside CM. We can easily decide, as a policy, that a function evaluation problem will throw a new, unchecked, "FunctionEvaluationException" (to be defined in the "exception" package). This would hugely lighten the CM code and, for users of API v3, it would just mean modifying one "import" statement (i.e. they won't have to touch any existing try/catch blocks).
> 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.
[jira] Commented: (MATH-397) Inconsistencies between
"optimization.univariate" and "optimization.general"
Posted by "Luc Maisonobe (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-397?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12898641#action_12898641 ]
Luc Maisonobe commented on MATH-397:
------------------------------------
OK to either remove or rename ConvergingAlgorithm.
For the interface setting max iteration and getting evaluation ... what about IterativeAlgorithm ? ;-)
Not sure about gradients. If you think a single combined counter is enough go for it. From a users point of view, this is simpler and I don't think much functionality would be lost with a single counter. I don't even know what a user could say if the number of functions evaluation was low and the number of gradients evaluation was high. It would be confusing but may happen.
OK for moving UnivariateRealOptimizer and for the new pair.
> 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.
[jira] Commented: (MATH-397) Inconsistencies between
"optimization.univariate" and "optimization.general"
Posted by "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.
[jira] Commented: (MATH-397) Inconsistencies between
"optimization.univariate" and "optimization.general"
Posted by "Gilles (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-397?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12897723#action_12897723 ]
Gilles commented on MATH-397:
-----------------------------
Actually, I think that the idea of "IterativeAlgorithm" is not useful, in the context of optimization, since it's internal to an algorithm's implementation and its meaning is algorithm-dependent: the objective measure of control is the number of evaluations. Hence I think that the maximal number of iterations should not be settable.
What should be part of the user-interface are methods for:
* setting maximal number of evaluations,
* getting the number of evaluations used.
I don't know what name to give to such an interface. Ideas?
Then, for algorithms that use the gradient, should the number gradient evaluations be considered as a separate setting or should it increment the same counter as the function evaluation? If the setting is used to compare the performance of various algorithms, I think that a combined counter should be preferred.
> 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.
[jira] Resolved: (MATH-397) Inconsistencies between
"optimization.univariate" and "optimization.general"
Posted by "Gilles (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-397?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Gilles resolved MATH-397.
-------------------------
Resolution: Duplicate
Final resolution is delegated to issue [MATH-413|https://issues.apache.org/jira/browse/MATH-413].
> 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: 3.0
>
>
> 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.
[jira] Commented: (MATH-397) Inconsistencies between
"optimization.univariate" and "optimization.general"
Posted by "Gilles (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-397?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12897757#action_12897757 ]
Gilles commented on MATH-397:
-----------------------------
The {{optimization}} package would be less cluttered if everything related to "univariate" would be located in the {{optimization.univariate}} package. E.g that would imply moving the {{UnivariateRealOptimizer}} interface. In there, I'll also create a {{UnivariateRealPointValuePair}} to be used by the "ConvergenceChecker" implementations. Is that OK?
> 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.