commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Evan Ward <>
Subject Re: [math] leastsquares refactoring
Date Fri, 14 Feb 2014 15:33:59 GMT

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:
>> 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,


> 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:
> For additional commands, e-mail:

To unsubscribe, e-mail:
For additional commands, e-mail:

View raw message