commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Phil Steitz <phil.ste...@gmail.com>
Subject Re: [math] replace AbstractContinuousDistribution.getSolverAbsoluteAccuracy() with AbstractContinuousDistribution.getSolver()
Date Sun, 06 Nov 2011 06:39:56 GMT
On 11/5/11 10:16 AM, Sébastien Brisard wrote:
>> 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.

That is a good point I had not thought of.  I agree that because of
this, we should not force creation of a solver in the base class.
>
>> 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
+1, but your observation above then leads to the question where are
you going to get this value from?  There may not be a solver to read
it from.  I guess the default impl in the base class could just
return BaseAbstractUnivariateRealSolver.DEFAULT_FUNCTION_VALUE_ACCURACY.

Phil
> Are you OK with that?
>
> Sébastien
>> Phil
>>> Sébastien
>>>
>>>
>>> ---------------------------------------------------------------------
>>> 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
>
>


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


Mime
View raw message