[
https://issues.apache.org/jira/browse/MATH-1658?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17736428#comment-17736428
]
François Laferrière edited comment on MATH-1658 at 6/23/23 10:41 AM:
---------------------------------------------------------------------
Correct. I stubble on this kind of of field initialization order problem a few
time and it is quite annoying.
There are other ways to settle ConvergenceChecker:
# make protected BaseOptimizer(ConvergenceChecker) deprecated and leave only
the default constructor BaseOptimizer(). Then make parseOptimizationData(...)
public so that optimizer creation is done in two steps - A - call constructor -
B - parse optimization data. This two steps process may be delegated to a
factory.
# rewrite the BaseOptimizer:parseOptimizationData so that it is private and
final, using reflexion and bean interface. The idea is to get the class name of
each optData (ex. MaxEval) and search a bean accessor with signature
set<className>(Class value).
Proposition 1 is very easy to implements.
Proposition 2 is a bit more tedious, because we will have to convert all
parseOptimizationData code (in all BaseOptimizer sub classes) into a bunch of
setter.
for instance
{code:java}
protected void parseOptimizationData(OptimizationData... optData) {
// The existing values (as set by the previous call) are reused if
// not provided in the argument list.
for (OptimizationData data : optData) {
if (data instanceof MaxEval) {
maxEvaluations = ((MaxEval) data).getMaxEval();
continue;
}
if (data instanceof MaxIter) {
maxIterations = ((MaxIter) data).getMaxIter();
continue;
}
if (data instanceof ConvergenceChecker) {
checker = (ConvergenceChecker<PAIR>) data;
continue;
}
}
}
{code}
is replaced by
{code:java}
protected void setMaxEval(MaxEval optData) {
maxEvaluations = optData.getMaxEval();
}
protected void setMaxIter(MaxIter optData) {
maxEvaluations = optData.getMaxIter();
}
protected void setConvergenceChecker(ConvergenceChecker<PAIR> optData) {
checker = optData;
}
{code}
As noted above, it is tedious because some code has to be touched in a score of
classes. On the other hand side, doing so we get rid of all the ugly instanceof
and type casts.
was (Author: JIRAUSER299106):
Correct. I stubble on this kind of of field initialization order problem a few
time and it is quite annoying.
There are other ways to settle ConvergenceChecker:
# make protected BaseOptimizer(ConvergenceChecker) deprecated and leave only
the default constructor BaseOptimizer(). Then make parseOptimizationData(...)
public so that optimizer creation is done in two steps - A - call constructor -
B - parse optimization data. This two steps process may be delegated to a
factory.
# rewrite the BaseOptimizer:parseOptimizationData so that it is private and
final, using reflexion and bean interface. The idea is to get the class name of
each optData (ex. MaxEval) and search a bean accessor with signature
set<className>(Class value).
Proposition 1 is very easy to implements.
Proposition 2 is a bit more tedious, because we will have to convert all
parseOptimizationData code (in all BaseOptimizer sub classes) into a bunch of
setter.
for instance
{{ protected void parseOptimizationData(OptimizationData... optData) {
// The existing values (as set by the previous call) are reused if
// not provided in the argument list.
for (OptimizationData data : optData) {
if (data instanceof MaxEval) {
maxEvaluations = ((MaxEval) data).getMaxEval();
continue;
}
if (data instanceof MaxIter) {
maxIterations = ((MaxIter) data).getMaxIter();
continue;
}
if (data instanceof ConvergenceChecker) {
checker = (ConvergenceChecker<PAIR>) data;
continue;
}
}
}
}}
is replaced by
{{ protected void setMaxEval(MaxEval optData) {
maxEvaluations = optData.getMaxEval();
}
protected void setMaxIter(MaxIter optData) {
maxEvaluations = optData.getMaxIter();
}
protected void setConvergenceChecker(ConvergenceChecker<PAIR> optData) {
checker = optData;
}
}}
As noted above, it is tedious because some code has to be touched in a score of
classes. On the other hand side, doing so we get rid of all the ugly instanceof
and type casts.
> ConvergenceChecker should implement OptimizationData
> ----------------------------------------------------
>
> Key: MATH-1658
> URL: https://issues.apache.org/jira/browse/MATH-1658
> Project: Commons Math
> Issue Type: Improvement
> Components: legacy
> Affects Versions: 4.0-beta1
> Reporter: François Laferrière
> Priority: Minor
> Attachments:
> MATH-1658-ConvergenceChecker-implements-OptimizationData.patch
>
>
> It is a bit peculiar that with BaseOptimizer, ConvergenceChecker can be
> passed only in constructor.
> It should be possible to change ConvergenceChecker at any other time.
> Further it is a bit strange that the constructor do not accept other
> OptimizationData.
> In my use cases, I need to create an optimizer once with complex
> OptimizationData and reuse it many time with very few changes (including
> changing the ConvergenceChecker).
> So, I suggest that
> * ConvergenceChecker implements OptimizationData.
> * protected BaseOptimizer(ConvergenceChecker<PAIR> checker) is replaced by
> protected BaseOptimizer(OptimizationData... optData)
> Doing so is compatible with existing code and provide much more flexibility.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
