On Mon, 26 Aug 2013 13:59:32 -0400, Evan Ward wrote:
Hi again,
I rearranged the least squares package and I've posted the
results.[1]
I've also created a pull request[2] and an associated issue.[3]
[1] https://github.com/wardev/commons-math/tree/sepOpt
[2] https://github.com/apache/commons-math/pull/1
[3] https://issues.apache.org/jira/browse/MATH-1026
A summary of what I changed: (See the git log for more details.)
Thanks for the effort!
Could you attach a patch to the issue page?
Hmm, actually, there would be so many changes that I don't think it's
really useful to have a patch.
Wouldn't it be clearer to create entirely new classes for everything,
in a new package? [Suggestions for a name?]
[Then we can do a "manual" diff for selected files to see how things
have evolved.]
Could you please accompany all the comments below with file names and
line numbers references?
1. Separate LeastSquaresProblem from LeastSquaresOptimizer. Update
tests
to use the new interfaces.
2. Immutable fluent builders for optimizers.
3. Remove weights from the LeastSquaresProblem interface. They are
pre-applied to the Jacobian and residuals.
4. Single function for computed values and Jacobian. I'm not 100% on
the
name. Call it MultivariateVectorMatrixFunction? or
MultivariateJacobianFunction? or
MultivariateVectorAndMatrixFunctionWithANameLongerThan80Characters?
:)
5. Make LevenbergMarquardt thread safe by using explicit temporary
parameters instead of instance fields.
6. In GaussNewton change useLU to an Enum
7. Change ConvergenceChecker<PointVectorValuePair> to
CovergenceChecker<Evaluation>. Would allow conditions like "RMS
changes
less than 1e-3".
8. Use [math] linear package in interfaces (RealMatrix and RealVector
instead of double[] and double[][])
I did not understand some things in the test code:
1. Why is AbstractLeastSquaresOptimizerTestValidation (renamed to
EvaluationTestValidation) disabled by default and how do I tell if it
passed?
Not sure that the rename is fine here.
The comments on the test methods explain the purpose; and the result
(plotting the output) should be examined "manually".
2. Other smaller questions about the tests. Theses are in comments in
the test package and marked with TODO.
Please ask them here, so that we can arrive to a common answer.
3. LU and QR use a default singularity threshold. Caused test
failures
when GaussNewton is configured to use QR. (MATH-1024)
You are looking for trouble.
The simpler route would be to do the conversion firs, ensuring that all
tests still pass.
Afterwards, new issues can be raised and resolved on their own.
4. A single test case in LevenburgMarquardt.testControlParameters()
now
converges where it used to diverge. It estimates a reasonable rms
after
15 iterations. I'm not sure why it used to diverge...
Same here. You might have uncovered a bug, or you may have introduced
one.
There are still several items on the TODO list. Code and discussion
is
appreciated. :)
Affecting the interface:
4. Immutable fluent builders for LSProblem?
6. maxIterations and ConvergenceChecker seem to have duplicate
functionality. (To stop the algorithm after a certain number of
iterations.) Remove the maxIterations parameter and let the
ConvergenceCheck handle that job.
No, the functionality is different. [There was a recent issue
about this.]
The optimizer is supposed to raise an exception when the iteration
count is exhausted, while the "ConvergenceChecker" can be used to
return the "best solution" found so far.
Implementation only:
2. Efficient diagonal weights. (don't need to create a new matrix,
just
multiply in place)
3. Add more convenience methods to Factory and Builder. E.g. for
diagonal weights, ...
4. Add conditional logic to Factory so it can use efficient diagonal
weights even if they are provided in a matrix.
3. Use [math] linear package in optimizer implementations. To match
the
current code in performance I think this would mean adding "views" or
specialized multiplication methods to the linear package. Depends on
MATH-765 and MATH-928.
Use of about-to-be-deprecated code in brand-new code might not be the
best move. [Even if the deprecation is not imminent...]
Thanks for looking over the proposal and reviewing it.
Thanks for taking the time to delve into the code,
Gilles
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
