commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Sébastien Brisard <>
Subject Re: [math] replace AbstractContinuousDistribution.getSolverAbsoluteAccuracy() with AbstractContinuousDistribution.getSolver()
Date Sat, 05 Nov 2011 17:16:00 GMT
> We are talking about two different things here - 0) configuring the
> solver and 1) reading its properties (in the present case, as part
> of internal implementation).
> Regarding configuration, the current setup where distribution
> constructors have optional inverse cum absolute accuracy arguments
> is good and should not be changed, IMO.  Users may have no idea what
> solver to use, but a good idea how accurate they want their inverse
> cum results to be, so we should keep that config option.  Allowing a
> fully configured solver to be passed in may be overkill, but if we
> want to add a protected constructor in the abstract base class and
> add implementations to all of the distributions to support it, I am
> OK with that.  Just don't drop the current simple constructors that
> either just accept defaults fully or allow the absolute accuracy to
> be passed in as a double.
Indeed, my idea was to keep a constructor with simple parameters for
the configuration of the default solver (Brent, in the present
implementation), alongside with a more versatile one.

I've had second thoughts about the whole thing, and came to
approximately the same conclusion as you. I've realized that
sub-classes might override inverseCumulativeProbability, and in some
cases, the new implementation of this method might do without a solver
at all (for very simple distributions). So why provide a solver at
construction time? In the present implementation, a new instance of
the default solver is created at each call.

> Regarding read access to the properties of the solver, another
> option is to just add a protected getSolver and be done with it.
> Alternatively, getFunctionValueAbsoluteAccuracy could be exposed
> either protected or public.  The getter for absolute accuracy in the
> distribution context really mirrors the optional constructor
> argument above.
> I guess my recommendation is to solve the problem we actually have
> with the simplest change, which is probably just a protected
> accessor for the solver.

I like the least change principle! For the reason explained above (a
solver might not always be meaningful), I would favour an accessor
like getSolverFunctionValueAccuracy(), though, instead of getSolver().
Although I don't really mind, since it would be protected anyway.

Are you OK with that?

> Phil
>> Sébastien
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail:
>> For additional commands, e-mail:
> ---------------------------------------------------------------------
> To unsubscribe, e-mail:
> For additional commands, e-mail:

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

View raw message