[
https://issues.apache.org/jira/browse/MATH-815?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13421040#comment-13421040
]
Gilles edited comment on MATH-815 at 7/24/12 12:02 AM:
-------------------------------------------------------
* Usage of "System.err" instead of raising an exception.
* I prefer "MathArrays.copyOf" instead of "System.arraycopy"
* Why do you store "inverseSigma" as "double[][]" and "sigma" as "RealMatrix"?
* In the method "getSigma" you return a "clone", which will perform a shallow
copy.
* Method {{density}} should probably be written as:
{code}
public double density(final double[] vals)
throws DimensionMismatchException {
if (vals.length != numColumns) {
throw new DimensionMismatchException(vals.length, numColumns);
}
final double kernel = getKernel(vals, mu, inverseSigma);
final double denom = FastMath.pow(2 * Math.PI, numColumns / 2) *
FastMath.sqrt(sigmaDeterminant)) *
FastMath.exp(kernel);
final double prob = 1d / denom;
return prob;
}
{code}
* All calls to "Math" methods should be replaced by the equivalent call to
"FastMath".
* Whenever possible, variables must be declared and assigned in the same
statement (cf. lines 222 and 229 in "MultivariateNormalDistribution.java").
* "private" methods must also be documented.
* Computation in lines 116-119 ("MultivariateNormalDistribution.java") is
probably more efficient if written as:
{code}
for (int row = 0; row < numColumns; row++) {
final double factor = FastMath.sqrt(sigmaEigenvalues[row]);
for (int col = 0; col < numColumns; col++) {
tmpMatrix.multiplyEntry(row, col, factor);
}
}
{code}
was (Author: erans):
* Usage of "System.err" instead of raising an exception.
* I prefer "MathArrays.copyOf" instead of "System.arraycopy"
* Why do you store "inverseSigma" as "double[][]" and "sigma" as "RealMatrix"?
* In the method "getSigma" you return a "clone", which will perform a shallow
copy.
* Method {{density}} should probably be written as:
{code}
public double density(final double[] vals)
throws DimensionMismatchException {
if (vals.length != numColumns) {
throw new DimensionMismatchException(vals.length, numColumns);
}
final double kernel = getKernel(vals, mu, inverseSigma);
final double denom = FastMath.pow(2 * Math.PI, numColumns / 2) *
FastMath.sqrt(sigmaDeterminant)) *
FastMath.exp(kernel);
final double prob = 1d / denom;
return prob;
}
{code}
* All calls to "Math" methods should be replaced by the equivalent call to
"FastMath".
* Whenever possible, variables must be declared and assigned in the same
statement (cf. lines 222 and 229 in "MultivariateNormalDistribution.java").
* "private" methods must also be commented.
* Computation in lines 116-119 ("MultivariateNormalDistribution.java") is
probably more efficient if written as:
{code}
for (int row = 0; row < numColumns; row++) {
final double factor = FastMath.sqrt(sigmaEigenvalues[row]);
for (int col = 0; col < numColumns; col++) {
tmpMatrix.multiplyEntry(row, col, factor);
}
}
{code}
> Multivariate Normal Distribution
> --------------------------------
>
> Key: MATH-815
> URL: https://issues.apache.org/jira/browse/MATH-815
> Project: Commons Math
> Issue Type: New Feature
> Reporter: Jared Becksfort
> Priority: Minor
> Attachments: mvn.tgz, patch
>
> Original Estimate: 1m
> Remaining Estimate: 1m
>
> I will submit a class for Multivariate Normal Distributions. Not sure if it
> will allow sampling initially.
> > Hello,
> >
> > I have implemented some classes for multivariate Normal distributions,
> > multivariate normal mixture models, and an expectation maximization fitting
> > class for the mixture model. I would like to submit it to Apache Commons
> > Math. I still have some touching up to do so that they fit the style
> > guidelines and implement the correct interfaces. Before I do so, I thought
> > I would at least ask if the developers of the project are interested in me
> > submitting them.
> >
> > Thanks,
> > Jared Becksfort
> Dear Jared,
> Yes, that would be very nice to have such an addition! Remember to also
> include unit tests (refer to the current ones for examples). The best would
> be to split a submission up into multiple minor ones, each covering a natural
> submission (e.g. multivariate Normal distribution in one submission), and
> create an issue as described at
> http://commons.apache.org/math/issue-tracking.html .
> If you run into any problems, please do not hesitate to ask on this mailing
> list.
> Cheers, Mikkel.
--
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
