-
Notifications
You must be signed in to change notification settings - Fork 3
Open
Description
Commit d108348 introduced a change that seems wrong to me, that is in the Analyse.doLRT method:
r.lrt = LikelihoodRatioTest.getSignificance(
r.models.get(1).lnL - r.models.get(0).lnL, // delta lnL
r.models.get(1).parameters - r.models.get(0).parameters); // degrees of freedombecame:
r.lrt = LikelihoodRatioTest.getSignificance(
Math.abs(r.models.get(1).lnL - r.models.get(0).lnL), // delta lnL
r.models.get(1).parameters - r.models.get(0).parameters); // degrees of freedomThe first version doesn't work when the optimization routine fails to find a better likelihood for r.models.get(1) than for r.models.get(0). The latter is bound to happen on some instances since the optimization problem cannot be solved in polynomial time. But the fix applied in commit d108348 is incorrect: in the case the complex model has a lesser likelihood than the simpler one, the discrepancy does not mean anything and does not follow the expected chi-square distribution. In this case, the program should report a convergence failure and not try to report a pvalue.
Metadata
Metadata
Assignees
Labels
No labels