commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gilles Sadowski <gil...@harfang.homelinux.org>
Subject Re: [Math] Cleanup in "HarmonicFitter" and "GaussianFitter"
Date Fri, 17 Aug 2012 14:59:42 GMT
On Fri, Aug 17, 2012 at 02:49:52PM +0100, sebb wrote:
> On 17 August 2012 14:41, Gilles Sadowski <gilles@harfang.homelinux.org> wrote:
> > On Fri, Aug 17, 2012 at 02:10:37PM +0100, sebb wrote:
> >> On 17 August 2012 13:49, Gilles Sadowski <gilles@harfang.homelinux.org>
wrote:
> >> > On Fri, Aug 17, 2012 at 01:24:50PM +0100, sebb wrote:
> >> >> On 17 August 2012 12:15, Gilles Sadowski <gilles@harfang.homelinux.org>
wrote:
> >> >> > On Fri, Aug 17, 2012 at 11:46:22AM +0100, sebb wrote:
> >> >> >> On 17 August 2012 11:24, Gilles Sadowski <gilles@harfang.homelinux.org>
wrote:
> >> >> >> > On Fri, Aug 17, 2012 at 09:40:49AM +0100, sebb wrote:
> >> >> >> >> On 17 August 2012 07:40, Luc Maisonobe <Luc.Maisonobe@free.fr>
wrote:
> >> >> >> >> > Le 16/08/2012 23:48, sebb a écrit :
> >> >> >> >> >> On 16 August 2012 21:58, Gilles Sadowski
<gilles@harfang.homelinux.org> wrote:
> >> >> >> >> >>> Hello.
> >> >> >> >> >>>
> >> >> >> >> >>> In classes "HarmonicFitter" and "GaussianFitter"
(in package
> >> >> >> >> >>> "o.a.c.m.optimization.fitting"), there
is an inner class "ParameterGuesser".
> >> >> >> >> >>> All the necessary input for the guessing
procedure is passed to the
> >> >> >> >> >>> constructor; I thus propose that the
guessing is performed at object
> >> >> >> >> >>> construction rather than at the call
to the "guess()" method. This will
> >> >> >> >> >>> allow to declare all the fields "final".
Moreover, all that can go wrong
> >> >> >> >> >>> will be located in the constructor,
turning the "guess" method into a mere
> >> >> >> >> >>> accessor.
> >> >> >> >> >
> >> >> >> >> > +1 to Gilles suggestion.
> >> >> >> >> >
> >> >> >> >> >>
> >> >> >> >> >> On the face of it, it sounds like the class
does not even need to be
> >> >> >> >> >> instantiated.
> >> >> >> >> >> Could the guess method be turned into a
static utility method to which
> >> >> >> >> >> the parameters are passed?
> >> >> >> >> >> Why bother creating an instance only to
return the guess later?
> >> >> >> >> >
> >> >> >> >> > It could be. There are a bunch of methods involved
and they exchange
> >> >> >> >> > data which is now stored in a few class fields
(observation arrays and
> >> >> >> >> > the three estimated parameters), so these data
should be added as
> >> >> >> >> > parameters to the static functions.
> >> >> >> >> >
> >> >> >> >> > I'm not sure whether it is cleaner to have a
small instance created for
> >> >> >> >> > a short time to perform some difficult preprocessing
or to have a bunch
> >> >> >> >> > of static methods.
> >> >> >> >>
> >> >> >> >> Instances need to be disposed of by GC whereas parameters
> >> >> >> >> automatically disappear when a called procedure returns.
> >> >> >> >
> >> >> >> > As a short-lived object, I guess that performance will
not be improved
> >> >> >> > greatly (especially w.r.t. the time taken for the fitting
itself).
> >> >> >> >
> >> >> >> >> A class with no fields is guaranteed immutable.
> >> >> >> >
> >> >> >> > The class will be immutable.
> >> >> >> >
> >> >> >> >>
> >> >> >> >> But for large numbers of parameters, class data is
perhaps cleaner.
> >> >> >> >
> >> >> >> > Being "public", the inner class cannot disappear now;
so I'll do the first
> >> >> >> > cleanup, and we'll see later if further changes are in
order.
> >> >> >>
> >> >> >> Is the class directly usable by 3rd parties?
> >> >> >
> >> >> > Yes.
> >> >> >
> >> >> >> Would it make sense for a 3rd party to use it directly?
> >> >> >
> >> >> > Maybe. They could compare the guessed values with the optimized
ones.
> >> >> > Admittingly, this is not very useful... if everything works fine.
> >> >> >
> >> >> >>
> >> >> >> If not, then although it would break strict binary compat.,
it would
> >> >> >> be possible to consider changing it.
> >> >> >> If it's not directly used by 3rd party code, I don't think
changes can
> >> >> >> break their code.
> >> >> >
> >> >> > I don't understand: the class is public. Someone can have instantiated
it in
> >> >> > his code...
> >> >>
> >> >> Not if the following is true:
> >> >>
> >> >> >> If it's not directly used by 3rd party code
> >> >
> >> > But you cannot know whether it is used or not.
> >>
> >> Yes, I can, because that is what is meant by:
> >>
> >> *If* it's not directly used by 3rd party code
> >>
> >> > Or am I missing something?
> >>
> >> Yes.
> >>
> >> Either the 3rd party code directly uses the class, or it does not.
> >> If not, it can only be used by non-3rd party code (if at all); i.e.
> >> only by Math.
> >> In which case, Math can change the implementation as it sees fit.
> >
> > This is a true statement for *any* code in the library: If it's not used, we
> > can change it (!).  But my question was and still is:  How do you know it's
> > not used?  I can, right now, trivially write an application that will
> > directly use that class.  Then I can claim that a minor release will have
> > broken backwards compatibility!
> >
> >> ===
> >>
> >> However, given that there does appear to be reason for 3rd party code
> >> to use the class, this is academic.
> >
> > Your argument could however be used for anything else (see below).
> >
> >>
> >> In other cases, it may turn out that the chance of 3rd party code
> >> using a class is vanishingly small.
> >> In which case it should be OK to break compat.
> >
> > What is "vanishingly small" here?
> 
> That the likelihood of it being used is very small, based on what the item does.
> 
> It's a risk judgement to be made by the developers - no hard and fast
> rules here.

There are changes we didn't perform on "equally" vanishingly small issues
(e.g. changing the value of a "public" default tolerance).
According to what you say here, we could have run the risk of changing those
"little" things.
But what if someone then complains (_after_ the release that breaks
compatibility)?

Sorry, but it does not make sense to have to make those "risky" judgements
whereas we can just make more releases (minor do not break anything, major
can break anything).
It does not make sense because it leads to discussions about how significant
a change is, which is completely subjective.

It does not make sense because, if someone used the code, say intantiated
the "HarmonicFitter.ParameterGuesser", and we now make it disappear, there
is a 100% chance that his code will be broken. For him that's not exactly
vanishingly small. ;-)

> 
> > Is polling the "user" ML about breaking something (say, in order to fix it),
> > and getting no objection, enough to carry on?
> 
> That would only be useful if a user said yes, they did use it.
> There's no guarantee that users of the item are even subscribed.
> 
> So a mailing list poll can only serve to veto a change.

Exactly what I meant to arrive at. We'll only agree not to break
compatibility. So let's stick to the simple rule...

> 
> > There are several issues
> > whose fix version is set to 4.0 which could be resolved right now if that's
> > the case.
> >
> >
> > Gilles
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> > For additional commands, e-mail: dev-help@commons.apache.org
> >
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message