[ 
https://issues.apache.org/jira/browse/MATH-817?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13609078#comment-13609078
 ] 

Gilles commented on MATH-817:
-----------------------------

Jared,

Please never mix formatting changes of existing source files together with code 
modifications. This is what you patch does for the file
"MixtureMultivariateRealDistribution". Moreover, in this case, the formatting 
changes were for the worse.
I've manually corrected a lot of the formatting strangeness (but I've not run 
it through Checkstyle yet).

More importantly, you've added "equals" and "hashCode" to 
"MixtureMultivariateRealDistribution", but "equals" defer further checks to a 
List<T> which itself will defer to the elements; but those elements 
(distribution instances) are not required to define "equals": thus the 
top-level "equals" is redefined to allow different instances to be equal but 
relies on a low-level "equals" that require instance equality.
However it seems only required to perform comparisons in some of the unit 
tests. If so, it would be much better to explicitly check what is required for 
the test to pass.
I'm going to commit the code but without "equals"; thus I'm setting the now 
failing test to "@Ignore". Please check out the test file and see whether what 
I've outlined is workable. Thanks.

                
> Multivariate Normal Mixture Model Fitting by Expectation Maximization
> ---------------------------------------------------------------------
>
>                 Key: MATH-817
>                 URL: https://issues.apache.org/jira/browse/MATH-817
>             Project: Commons Math
>          Issue Type: New Feature
>            Reporter: Jared Becksfort
>            Priority: Minor
>         Attachments: math_817.patch, 
> MultivariateNormalMixtureExpectationMaximizationFitter.java, 
> MultivariateNormalMixtureExpectationMaximizationFitterTest.java
>
>   Original Estimate: 1m
>  Remaining Estimate: 1m
>
> I will submit a class for fitting Multivariate Normal Mixture Models using 
> Expectation Maximization.
> > 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
For more information on JIRA, see: http://www.atlassian.com/software/jira

Reply via email to