-
Notifications
You must be signed in to change notification settings - Fork 28.6k
[SPARK-19155][ML] Make family case insensitive in GLM #16675
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
Conversation
I would prefer that |
@actuaryzhang I think the change is not appropriate, the function |
@yanboliang Thanks for the quick response. How about the new commit, where I just change the value from |
Looks good, I'll merge if it passes test. Thanks. |
Test build #71819 has finished for PR 16675 at commit
|
Test build #71820 has finished for PR 16675 at commit
|
@yanboliang Thanks. Seems to have passed tests. |
## What changes were proposed in this pull request? This is a supplement to PR #16516 which did not make the value from `getFamily` case insensitive. Current tests of poisson/binomial glm with weight fail when specifying 'Poisson' or 'Binomial', because the calculation of `dispersion` and `pValue` checks the value of family retrieved from `getFamily` ``` model.getFamily == Binomial.name || model.getFamily == Poisson.name ``` ## How was this patch tested? Update existing tests for 'Poisson' and 'Binomial'. yanboliang felixcheung imatiach-msft Author: actuaryzhang <actuaryzhang10@gmail.com> Closes #16675 from actuaryzhang/family. (cherry picked from commit f067ace) Signed-off-by: Yanbo Liang <ybliang8@gmail.com>
LGTM, merged into master and branch-2.1. Thanks. |
## What changes were proposed in this pull request? This is a supplement to PR apache#16516 which did not make the value from `getFamily` case insensitive. Current tests of poisson/binomial glm with weight fail when specifying 'Poisson' or 'Binomial', because the calculation of `dispersion` and `pValue` checks the value of family retrieved from `getFamily` ``` model.getFamily == Binomial.name || model.getFamily == Poisson.name ``` ## How was this patch tested? Update existing tests for 'Poisson' and 'Binomial'. yanboliang felixcheung imatiach-msft Author: actuaryzhang <actuaryzhang10@gmail.com> Closes apache#16675 from actuaryzhang/family.
## What changes were proposed in this pull request? This is a supplement to PR apache#16516 which did not make the value from `getFamily` case insensitive. Current tests of poisson/binomial glm with weight fail when specifying 'Poisson' or 'Binomial', because the calculation of `dispersion` and `pValue` checks the value of family retrieved from `getFamily` ``` model.getFamily == Binomial.name || model.getFamily == Poisson.name ``` ## How was this patch tested? Update existing tests for 'Poisson' and 'Binomial'. yanboliang felixcheung imatiach-msft Author: actuaryzhang <actuaryzhang10@gmail.com> Closes apache#16675 from actuaryzhang/family.
What changes were proposed in this pull request?
This is a supplement to PR #16516 which did not make the value from
getFamily
case insensitive. Current tests of poisson/binomial glm with weight fail when specifying 'Poisson' or 'Binomial', because the calculation ofdispersion
andpValue
checks the value of family retrieved fromgetFamily
How was this patch tested?
Update existing tests for 'Poisson' and 'Binomial'.
@yanboliang @felixcheung @imatiach-msft