[
https://issues.apache.org/jira/browse/MATH-703?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13157001#comment-13157001
]
Sébastien Brisard commented on MATH-703:
----------------------------------------
Hi Christian,
I've reviewed your patch, and have some concerns
* in class {{AbstractRealDistribution}}, the mean and variance of the actual
distribution is cached, which is good, but I think that nothing guarantees
immutability of classes inheriting {{AbstractRealDistribution}}. For example, I
might implement my own normal distribution, with {{setMean(double)}} and
{{setVariance(double)}} methods, and I would then need to make sure the proper
private fields in the {{AbstractRealDistribution}} are updated accordingly.
While caching should be allowed for in this hierarchy of classes, I think no
default implementation of {{getNumericalMean()}} and {{getNumericalVariance()}}
should be provided at this stage. This might be silly, please correct me if I'm
wrong. I think people raised the same concerns about the dimensions of the
domain and codomain of linear operators, when I first submitted an
implementation where these dimensions where stored in the base abstract class.
* I'm not convinced about {{AbstractDefaultRealDistribution}} and
{{AbstractDefaultIntegerDistribution}}: not having them would only mean that we
would need to implement one or two additional methods, with very simple (in
fact, constant, as you noticed) values. So we need to compare this slight
inconvenience on the one hand with the encumbrance of the class hierarchy on
the other hand... And in any case, people would *always* have to refer to the
Javadoc of these default implementations to make sure that what they want to
implement is indeed a {{AbstractDefaultXxxDistribution}}
These are only my views, though, and I'm going to post an invitation on the
mailing-list in order to invite others to review your work.
> Splitting up the distribution hierarchy
> ---------------------------------------
>
> Key: MATH-703
> URL: https://issues.apache.org/jira/browse/MATH-703
> Project: Commons Math
> Issue Type: Improvement
> Reporter: Christian Winter
> Priority: Minor
> Attachments: MATH-703_patch.zip
>
>
> As discussed on the mailing list
> (http://apache-commons.680414.n4.nabble.com/math-Distributions-over-sample-spaces-other-than-R-tp3931349p3931349.html),
> the distribution interfaces should be restructured.
> The most important point is to create one root interface for each domain.
> There should *not* be a common super-interace because different domains
> require different functionality. Additionally, a super-inferface would
> require to parametrize the domain which makes things more complicated (e.g.,
> "double" would have to be replaced by "Double"). Currently, Commons Math
> supports distributions with real domain and distributions with integer
> domain. Thus there will be the interfaces RealDistribution and
> IntegerDistribution.
> Another point is to drop the special cases of distributions with real domain
> in order to simplify the structure. There won't be an interface for
> absolutely continuous distributions, and there won't be an interface for
> discrete distributions on the real domain. All the functionality required by
> the special cases can be defined in RealDistribution.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira