-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PUBDEV-8947: aic glm #6672
PUBDEV-8947: aic glm #6672
Conversation
…oving duplicate calculation
…un - removing duplicate calculation" This reverts commit f9627e8.
…rom GLMParameters
@syzonyuliia : I have added a R unit test for you that attempts to compare likelihood/aic calculation for GLM. You can do a |
…rom GLMParameters
R.formula = (R.data[,"GLEASON"]~.) | ||
model.h2o.gaussian.identity <- h2o.glm(calc_like=TRUE, x=myX, y=myY, training_frame=h2o.data, family="gaussian", link="identity",alpha=0.5, lambda=0, nfolds=0) | ||
model.R.gaussian.identity <- glm(formula=R.formula, data=R.data[,2:9], family=gaussian(link=identity), na.action=na.omit) | ||
perf <- h2o.performance(model.h2o.gaussian.identity, h2o.data) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
change
perf <- h2o.performance(model.h2o.gaussian.identy, h2o.data)
to
perf <- h2o.performance(model.h2o.gaussian.identy, train=TRUE)
h2o-r/tests/testdir_algos/glm/runit_PUBDEV_8947_GLM_likelihoods_aic.R
Outdated
Show resolved
Hide resolved
h2o-r/tests/testdir_algos/glm/runit_PUBDEV_8947_GLM_likelihoods_aic.R
Outdated
Show resolved
Hide resolved
h2o-r/tests/testdir_algos/glm/runit_PUBDEV_8947_GLM_likelihoods_aic.R
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most are only small changes.
…ative_binomial' into PUBDEV-8947-AIC-GLM
… dispersion parameter
- log(yr) - Gamma.logGamma(w * invPhiEst); | ||
case tweedie: | ||
if (DispersionMethod.ml.equals(_dispersion_parameter_method) && !_fix_tweedie_variance_power) { | ||
return -TweedieVariancePowerMLEstimator.logLikelihood(yr, ym[0], _tweedie_variance_power, _init_dispersion_parameter); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change
if (DispersionMethod.ml.equals(_dispersion_parameter_method) && !_fix_tweedie_variance_power)
to just
if (DispersionMethod.ml.equals(_dispersion_parameter_method))
We should not care what the user sets when they want to estimate Tweedie dispersion parameter. They should be able to get their likelihood calculation regardless.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh right, and we also explicitly set the _dispersion_parameter_method to ML, so there is no reason to check for it
https://h2oai.atlassian.net/browse/PUBDEV-8947