[
https://issues.apache.org/jira/browse/MATH-1658?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17707890#comment-17707890
]
François Laferrière edited comment on MATH-1658 at 4/3/23 11:04 AM:
--------------------------------------------------------------------
I am not sure that optim package should follow the same pattern as
fitting.leastsquare. Indeed I was a bit surprised (to say the less) by the
extensive use "instanceof" in optim package. On the other hand side, I am not
convinced at all by the radical immutability of the leastsquare package. It is
due to my personal use cases. I use optim in a Digital Signal Processing
application where the Optimizer...optimize(...) is called literally millions on
time with tiny changes to parameters each time. So with my use case,
reallocating a new Optimizer each time may be an issue.
Another point that LeastSquaresOptimizer implementations (Gauss-Newton and
Levenberg-Marquard) have a relatively small number of constructor parameters.
LevenbergMarquardtOptimizer main constructor have 5 double parameters which is
close to the limit beyond wich API is error prone (especially if many
parameters are of the same type).
Optimizer implementation can have often more than 10 parameters and have many
of them with default value. So the current API entailing a variable length list
of OptimizationData seems (at least to me) more practical and less error prone.
Nevertheless we could borrow from leastsquare package the idea of a separation
between the Optimizer (that may be immutable) from the Problem (which is a
smaller object that causes less allocation and parsing issues)
So I suggest, as a first step, to stick to the old interface for optim, and
just make ConvergenceChecker consistent with the rest of OptimizationData.
In another step, we may think of a separation of the Optimizer from the Problem.
was (Author: JIRAUSER299106):
I am not sure that optim package should follow the same pattern as
fitting.leastsquare. Indeed I was a bit surprised (to say the less) by the
extensive use "instanceof" in optim package. On the other hand side, I am not
convinced at all by the radical immutability of the leastsquare package. It is
due to my personal use cases. I use optim in a Digital Signal Processing
application where the Optimizer...optimize(...) is called literally millions on
time with tiny changes to parameters each time. So with my use case,
reallocating a new Optimizer each time may be an issue.
Another point that LeastSquaresOptimizer implementations (Gauss-Newton and
Levenberg-Marquard) have a relatively small number of constructor parameters.
LevenbergMarquardtOptimizer main constructor have 5 double parameters which is
close to the limit beyond wich API is error prone (especially if many
parameters are of the same type).
Optimizer implementation can have often more than 10 parameters and have many
of them with default value. So the current API entailing a variable length list
of OptimizationData seems (at least to me)à more practical and less error prone.
So I suggest to stick to the old interface for optim, and just make
ConvergenceChecker consistent with the rest of OptimizationData.
> 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
>
> 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)
