On 08/13/2013 08:17 AM, Gilles wrote: >> (I.e. GN would >> not need 5 superclasses and fields for storing upper and lower bounds.) > > Be careful: "GaussNewtonOptimizer" does _not_ support bounds!
That is my point! Because of it's complex inheritance every instance includes fields for storing bounds even though the algorithm has no use for them. (Declared in BaseMultivariateOptimizer) I was looking at the released (3.2) version. Looks like it has since been fixed. :) > >> >> With the API re-design you have the opportunity to design for thread >> safety, which can be hard to add on to an existing API. > > True in general, but in this case we didn't make a thorough analysis > of advantages and drawbacks of creating a new instance for each > application of a "withXxx" method. > I previously hinted that such a behaviour might not be desirable, > independently of the issue of thread-safety. > >> Thanks for the whole discussion; It has given me some new ideas. :) > > You are most welcome to experiment with the current revision of the > code in "o.a.c.m.fitting.leastsquares", in order to pinpoint problems > for your use-cases. > > You could also provide the concrete changes (patch) needed to move the > "data" to another object, as you suggested. > If it's better design and leads to simpler code, there shouldn't be > any problem in applying it for the next release. I separated the algorithm from the problem definition for the GaussNewton optimizer. See https://gist.github.com/wardev/dfd16ac0ad3ba62ede05 If we decide to go this route LM would follow the same pattern. It still needs some polishing and a few smaller API decisions. A summary of the changes: - The two superclasses of GN were turned in to AbstractOptimizationProblem and LeastSquaresProblemImpl. I tried to keep most of the code the same so that a diff would be meaningful. - Interfaces were extracted from LeastSquaresProblemImpl and AbstractOptimizationProblem. I kept the upper level of the hierarchy as an implementation aid. - GaussNewton implements LeastSquaresOptimizer and does not have a super class. Evaluation and iteration counts are now locals. - There is now one method to evaluate the model/problem, see LeastSquaresProblem.evaluate(double[]). This returns a container with the value and Jacobian. Advantages: - GaussNewton is now trivially easy to make immutable and fluent and thread-safe. This will allow the same algorithm to be applied to different problems. - If the model function and convergence checker are thread safe, then so is the LeastSquaresProblem (if you don't mutate it). This would allow different algorithms to be applied to the same problem. These are the two big bullet points that I think make it worthwhile. :) - Allows algorithms to be applied to problems they weren't designed for by using "views". I.e. view a least squares problem as a direct problem by creating an implementation of DirectProblem that delegates to a LeastSquaresProblem. A similar delegation would work to view a DirectOptimzier as a LeastSquaresOptimizer. - Same code for the most part, just reorganized. Further decisions: - It would be nice to have a model interface that could be evaluated with one method call. E.g. Pair<double[], double[][]> value(double[]). - Leave getWeights() out of the LeastSquaresProblem API? This would help encapsulation, but would require further modification of the GN implementation. - Should the LeastSquaresProblem interface include mutators? I think it makes sense to only include the methods that the optimization algorithm needs to "read". That would keep the API smaller and not tie the users to a particular implementation. - method and class names. I'm sure there are a few more issues. :) Thanks for considering the separate API. Regards, Evan --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org