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

Gilles commented on MATH-878:
-----------------------------

About the patch, I'm only able to suggest "cosmetic" improvements:
* The comments are not easy to read:
** Enumerated lists should appear as such in the Javadoc (for developers' sake)
** <p>...</p> should be avoided as they almost never necessary (I use <br/> if 
a new paragraph is really needed.
** I prefer {@code ...} over <code>...</code> (whenever there are no other HTML 
tags inside them)
** Sometimes you use "The" as the first word of the description for "@param", 
sometimes not. (I prefer to always skip it).
** Often there is no text for <a> tags: the (sometimes ugly) link will appear 
in the processed apidocs. (And they are not closed with a </a>.)
** Comments should preferably end with a period (".").
** The Javadoc main description should always come before the various Javadoc 
tags (i.e. not partly before and partly after).
** I'm wary of the doc to contain links to non-widely used web sites (e.g. 
blogs and mailing lists archives). (AFAIK no other CM code does this so that it 
was never discussed whether this is allowed or not.) In any case, it would be 
fine to just provide an inline summary of the conclusions, and possibly provide 
the links on the bug tracking system's page of the issue.
** The "@version" tag should read "@version $Id$".
* About the code formatting:
** Some weird alignments.
** An empty constructor is no necessary.
** Redundant sets of parentheses.
** Writing a double constant as "0.0d" is redundant: Either "0.0" or "0d". (I 
prefer the latter).
And, in a statement where the doubles are already involved, "0" is even clearer 
(IMO).
** "gTestGoodnessOfFit_pValue" is not a "standard" method name because of the 
underscore.
** Missing "final" keywords.
** Additions to "TestUtils" might come as a separate patch (first introduced 
the new functionality, then use it in other parts of CM).
** Name of the unit test methods: the underscore should be removed.

Sorry for the long list, which is certainly the result of _our_ failure to 
agree on the need to provide comprehensive guidelines to formatting!

                
> G-Test (Log-Likelihood ratio - LLR test) in math.stat.inference
> ---------------------------------------------------------------
>
>                 Key: MATH-878
>                 URL: https://issues.apache.org/jira/browse/MATH-878
>             Project: Commons Math
>          Issue Type: New Feature
>    Affects Versions: 3.1, 3.2, 4.0
>         Environment: Netbeans
>            Reporter: Radoslav Tsvetkov
>              Labels: features, test
>             Fix For: 3.1
>
>         Attachments: MATH-878_gTest_12102012.patch, 
> MATH-878_gTest_15102012.patch, vcs-diff16294.patch
>
>   Original Estimate: 24h
>  Remaining Estimate: 24h
>
> 1. Implementation of G-Test (Log-Likelihood ratio LLR test for independence 
> and goodnes-of-fit)
> 2. Reference: http://en.wikipedia.org/wiki/G-test
> 3. Reasons-Usefulness: G-tests are tests are increasingly being used in 
> situations where chi-squared tests were previously recommended. 
> The approximation to the theoretical chi-squared distribution for the G-test 
> is better than for the Pearson chi-squared tests. In cases where Observed 
> >2*Expected for some cell case, the G-test is always better than the 
> chi-squared test.
> For testing goodness-of-fit the G-test is infinitely more efficient than the 
> chi squared test in the sense of Bahadur, but the two tests are equally 
> efficient in the sense of Pitman or in the sense of Hodge and Lehman. 

--
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