You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Phil Steitz <ph...@gmail.com> on 2010/01/24 05:58:56 UTC
Re: [MATH] Density functions for all continuous distributions gives
rise to re-consider the HasDensity-interface
Mikkel Meyer Andersen wrote:
> Dear Community.
>
> I've just been going through all the continuous distributions in the
> MATH-library to check which implemented the HasDensity<P>-interface.
>
> Only the F-distribution, t-distribution, Weibull distribution, and
> Cauchy distribution didn't implement it, and with MATH-330
> (https://issues.apache.org/jira/browse/MATH-330 - patch for the
> F-distribution) and MATH-331
> (https://issues.apache.org/jira/browse/MATH-331 - patches for the
> rest) both made by me that should now be corrected.
>
> This means that now every continuous distribution implements
> HasDensity<Double>. Wouldn't it then be a good idea to put a double
> density(Double x)-method in the ContinuousDistribution-interface? It
> would give a lot of advantages, e.g. in regards to dynamic
> dispatching.
>
> What do you think?
Great work!
I will complete review and commit if someone does not beat me to it.
Technically, adding to the ContinuousDistribution interface is a
compatibility break, so should wait until 3.0; but if we follow
Ted's suggestion of adding a default impl to
AbstractContinuousDistribution, the only users actually impacted
would be those who have implemented distributions that do not derive
from AbstractContinuousDistribution. I wonder how many of these
there are.
I am +1 on this change, but understand if others are not comfortable
with the compatibility break.
Phil
>
> Cheers, Mikkel.
>
> ---------------------------------------------------------------------
> 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
Re: [MATH] Density functions for all continuous distributions gives
rise to re-consider the HasDensity-interface
Posted by Phil Steitz <ph...@gmail.com>.
Mikkel Meyer Andersen wrote:
> I haven't added to the ContinuousDistribution-interface. I've only
> added to the AbstractContinuousDistribution-class. The relevant patch
> that I committed is https://issues.apache.org/jira/browse/MATH-332, so
> no compatibility break should occur.
Yes, so your proposal is then to keep, but deprecate HasDensity and
leave the ContinuousDistribution interface alone; possibly
documenting the fact that as of 3.0, densities will be required.
Sounds reasonable.
Phil
>
> 2010/1/24 Phil Steitz <ph...@gmail.com>:
>> Mikkel Meyer Andersen wrote:
>>> Dear Community.
>>>
>>> I've just been going through all the continuous distributions in the
>>> MATH-library to check which implemented the HasDensity<P>-interface.
>>>
>>> Only the F-distribution, t-distribution, Weibull distribution, and
>>> Cauchy distribution didn't implement it, and with MATH-330
>>> (https://issues.apache.org/jira/browse/MATH-330 - patch for the
>>> F-distribution) and MATH-331
>>> (https://issues.apache.org/jira/browse/MATH-331 - patches for the
>>> rest) both made by me that should now be corrected.
>>>
>>> This means that now every continuous distribution implements
>>> HasDensity<Double>. Wouldn't it then be a good idea to put a double
>>> density(Double x)-method in the ContinuousDistribution-interface? It
>>> would give a lot of advantages, e.g. in regards to dynamic
>>> dispatching.
>>>
>>> What do you think?
>> Great work!
>>
>> I will complete review and commit if someone does not beat me to it.
>>
>> Technically, adding to the ContinuousDistribution interface is a
>> compatibility break, so should wait until 3.0; but if we follow
>> Ted's suggestion of adding a default impl to
>> AbstractContinuousDistribution, the only users actually impacted
>> would be those who have implemented distributions that do not derive
>> from AbstractContinuousDistribution. I wonder how many of these
>> there are.
>>
>> I am +1 on this change, but understand if others are not comfortable
>> with the compatibility break.
>>
>> Phil
>>
>>
>>> Cheers, Mikkel.
>>>
>>> ---------------------------------------------------------------------
>>> 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
Re: [MATH] Density functions for all continuous distributions gives
rise to re-consider the HasDensity-interface
Posted by Mikkel Meyer Andersen <mi...@mikl.dk>.
I haven't added to the ContinuousDistribution-interface. I've only
added to the AbstractContinuousDistribution-class. The relevant patch
that I committed is https://issues.apache.org/jira/browse/MATH-332, so
no compatibility break should occur.
2010/1/24 Phil Steitz <ph...@gmail.com>:
> Mikkel Meyer Andersen wrote:
>> Dear Community.
>>
>> I've just been going through all the continuous distributions in the
>> MATH-library to check which implemented the HasDensity<P>-interface.
>>
>> Only the F-distribution, t-distribution, Weibull distribution, and
>> Cauchy distribution didn't implement it, and with MATH-330
>> (https://issues.apache.org/jira/browse/MATH-330 - patch for the
>> F-distribution) and MATH-331
>> (https://issues.apache.org/jira/browse/MATH-331 - patches for the
>> rest) both made by me that should now be corrected.
>>
>> This means that now every continuous distribution implements
>> HasDensity<Double>. Wouldn't it then be a good idea to put a double
>> density(Double x)-method in the ContinuousDistribution-interface? It
>> would give a lot of advantages, e.g. in regards to dynamic
>> dispatching.
>>
>> What do you think?
>
> Great work!
>
> I will complete review and commit if someone does not beat me to it.
>
> Technically, adding to the ContinuousDistribution interface is a
> compatibility break, so should wait until 3.0; but if we follow
> Ted's suggestion of adding a default impl to
> AbstractContinuousDistribution, the only users actually impacted
> would be those who have implemented distributions that do not derive
> from AbstractContinuousDistribution. I wonder how many of these
> there are.
>
> I am +1 on this change, but understand if others are not comfortable
> with the compatibility break.
>
> Phil
>
>
>>
>> Cheers, Mikkel.
>>
>> ---------------------------------------------------------------------
>> 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