On 02/13/2014 12:19 PM, Gilles wrote: > Hello Luc. > > On Thu, 13 Feb 2014 16:10:36 +0100, Luc Maisonobe wrote: >> Hi Gilles, >> >> Le 08/02/2014 22:16, Gilles a écrit : >> >>>> Other issues to be discussed: >>>> >>>> * [MATH-1008] >>>> >>>> a new package has been created o.a.c.m.fitting.leastsquares which >>>> contains some interface for the new fluent API but they are also >>>> used elsewhere so I guess it would be more logical to put them in a >>>> more general place, e.g. WithMaxEvaluations should be moved to >>>> o.a.c.m.optim. WDYT? >>> >>> Strictly speaking, these have potential use outside the "o.a.c.m.optim" >>> package. >>> >>> Also, further improvements were postponed since the alternate proposal >>> looked quite promising: >>> https://issues.apache.org/jira/browse/MATH-1026 >>> >> >> I had a look at Evan's poposal in MATH-1026. I really like it! >> >> Do you think we could replace our current lestsquares (which has never >> been released yet), with this new one and finish it? > > In principle yes (see my first comment on the JIRA page). In practice, > I'm > not sure. > >> Concerning the last >> comment by Evan in the JIRA system, I think they could be answered >> quickly. Also the questions about caching and lazy evaluations are >> implementation details that could be changed later on, even after 3.3 >> release. > > My memory about all the discussions fades away. IIRC, the main problem > with the proposed code is that it provided supposed enhancements to the > functionality before showing that it could faithfully reproduce the > existing functionality (albeit with another API). > Because I could not be sure of the equivalence and did not have time to > make changes and checks, I did not want to take the responsibility for > committing the code in that uncertain state. > Maybe it's fine; I just don't know.
I would just like to add that I made a significant effort to ensure the API update is in a separate commit from all the other improvements. Commit 7e9a15e [1] contains the API switch. As with any API change it involved updating the test cases, so I realize there is a lot of code to read to check for correctness. I did factor common test code into reusable methods, but IMHO this should make it easier to review since there is only one opportunity for a typo instead of dozens. All the following commits in the pull request contain the functionality improvements and code quality improvements (as well as moving to a different package and test style changes). Please let me know if you have any questions about the code or about why I made a particular change. Best Regards, Evan [1] https://github.com/wardev/commons-math/commit/7e9a15efb535b91afad9d8275eb2864ea2295ab4 > Also, please take into account that, additionally to the API change for > the code now in "fitting.leastsquares", there has been some "little" > improvements in the code itself (e.g. compare the implementations of > "LevenbergMarquardtOptimizer" in "optim" vs "fitting.leastsquares"). > Before trashing this new code, it would be good to make sure that those > (tested) improvements have been retained in Evan's code. > To sum up, what is in "fitting.leastsquares" should be the reference > code, in the sense that post-release 3.3, _nothing_ from the deprecated > "optimization" and "optim.nonlinear.vector" should make its way to code > for 4.0. > Hence, I'd favour keeping the current "fitting.leastsquares" until we are > positively sure that everything there was indeed ported to Evan's code > (whenever applicable). > We could put both in an "experimental" sub-package; that would not delay > a 3.3 release, while leaving room for non-compatible changes in a 3.4 > release. > > > Best regards, > Gilles > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] > --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
