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-unsubscr...@commons.apache.org >>> For additional commands, e-mail: dev-h...@commons.apache.org >>> >>> >> >> --------------------------------------------------------------------- >> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org >> For additional commands, e-mail: dev-h...@commons.apache.org >> >> > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
