commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Gilles (JIRA)" <>
Subject [jira] [Commented] (MATH-878) G-Test (Log-Likelihood ratio - LLR test) in math.stat.inference
Date Sat, 20 Oct 2012 10:58:12 GMT


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

View raw message