Hi Luc.
[...]
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.
Looking at the code, it really seems Evan started from your
fitting.leastsquares (in particular, the InternalData internal class
introduced in your class is also in Evan's class).
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.
I don't like this. We has two versions of LevenbergMarquardt in
previous
version, and by introducing both fitting.leastsquares and
fitting.leastsquares2 would mean we have four implementations! I
really
think fitting.leastsquares should retain only one, and as the one
from
Evan was started from the one you did, it is the more advanced. So I
would select this one.
I also am quite reluctant to a 3.4 release. We should really move
forward and clean up old stuff, which can be done only by pushing a
4.0
release after 3.3.
So if nobody complains, I will start updating leastsquares with the
code
from MATH-1026, perhaps starting directly from the commits in the git
repository if possible. As I use git-svn as my tool of choice, this
should be achievable.
Hmm, please take into account that "fitting.leastsquares" was meant as
a
sort of proof-of-concept for a new API (mainly, the fluent API which
you
proposed) for _all_ optimizers. If 3.3 is to be released with the
current
trunk (modulo your pending changes in "fitting.leastsquares"), users
won't
have any transition period for all the other optimizers which are to be
removed in 4.0 (unless we keep the old cruft for another generation,
which
would be contrary to your stated goal of clean-up).
Hence either a 3.4 release is necessary to complete the API change, or
3.3
must be delayed until all the optimizers are modified similarly to
Evan's
proposal. But in this latter case, the change would happen even before
actually allowing users to provide feedback on the new API (by testing
it
on their least-squares use-cases)...
Regards,
Gilles
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
