Adding geometric distribution for GLM.jl #480

ayushpatnaikgit · 2022-04-22T11:27:21Z

The following is the summary of the changes in the following source files:

In docs/src/index.md:

Added Geometric to the list of distribution in family, under the “Fitting GLM models” section

In src/GLM.jl:

Exported Geometric

In src/glmtools.jl:

Defined LogLink as canonical link function for Geometric distribution
Defined glmvar function for Geometric distribution
Defined mustart function for Geometric distribution
Defined devresid function for Geometric distribution
Defined loglik_obs function for Geometric distribution
Added an example glmvar in docstring
Added an example mustart in docstring

In test/runtests.jl:

Added a test case of Geometric distribution with LogLink
Added a test case of Geometric distribution with InverseLink

…t, etc. The probability that a batsman can make a successful hit on the ball before he gets out by the bowler, is calculated efficiently using the geometric probability distribution function. Now each player will have their own distribution depending on their batting ability. So, the probability of successfully playing a ball can be modelled as a function of several covariates of the player. Note that, Geometric distribution is a special case Negative Binomial distribution when the number of failures is 1.

…ace `approx` function by `≈` in test cases.

nalimilan

Thanks! Have you checked the results against another implementation?

src/glmtools.jl

test/runtests.jl

docs/src/index.md

Co-authored-by: Milan Bouchet-Valat <nalimilan@club.fr>

codecov-commenter · 2022-04-25T05:51:22Z

Codecov Report

Merging #480 (633ed64) into master (42a0d04) will increase coverage by 0.16%.
The diff coverage is 94.11%.

@@            Coverage Diff             @@
##           master     #480      +/-   ##
==========================================
+ Coverage   85.12%   85.28%   +0.16%     
==========================================
  Files           7        7              
  Lines         827      836       +9     
==========================================
+ Hits          704      713       +9     
  Misses        123      123

Impacted Files	Coverage Δ
src/GLM.jl	`50.00% <ø> (ø)`
src/glmtools.jl	`83.80% <94.11%> (+1.09%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 42a0d04...633ed64. Read the comment docs.

palday · 2022-04-26T04:48:40Z

src/glmtools.jl

 devresid(::Gamma, y, μ::Real) = -2 * (log(y / μ) - (y - μ) / μ)
+function devresid(::Geometric, y, μ::Real)
+    v = 2 * (xlogy(y, y / μ) + xlogy(y + 1, (μ + 1) / (y + 1)))
+    return μ == 0 ? oftype(v, NaN) : v


I would probably move this check above to shortcircuit faster when mu==0.

Right - I will update the code similar to the following:
μ == 0 ? NaN : 2 * (xlogy(y, y / μ) + xlogy(y + 1, (μ + 1) / (y + 1)))

μ == 0 && return oftype(μ, NaN) return 2 * (xlogy(y, y / μ) + xlogy(y + 1, (μ + 1) / (y + 1)))

is probably how I would do it, but it's really a matter of personal style and what @nalimilan thinks 😄

I agree it sounds better to adapt the NaN type to that of the inputs. But I'm not sure whether it should be the type of μ, that of y, or promote_type(typeof(μ), typeof(y))...

promote_type(typeof(μ), typeof(y)) seems the way to go. Given that we have an expression with y/mu and xlogy never returns a narrower type than the inputs, we can expect the result to be at least as wide as y/mu.

Actually promote_type(typeof(μ), typeof(y)) will give a failure if both types are Integer as conversion of NaN won't work. So we need to do float(promote_type(typeof(μ), typeof(y))).

src/glmtools.jl

palday

It would be good to document where the numbers in the "Geometric LogLink" test set come from, but otherwise looks good to me.

nalimilan · 2022-04-26T08:10:30Z

It would be good to document where the numbers in the "Geometric LogLink" test set come from

I don't think we note this in the code usually, I think the implicit assumption is that values have been checked against R, either directly or indirectly (here, by checking against NegativeBinomial which was previously checked against R).

ViralBShah · 2022-05-19T17:31:34Z

Bumping this - what remains to merge?

nalimilan

Sorry, we kind of forgot about this. We just need to fix the discussion about the devresid return type.

src/glmtools.jl

nalimilan · 2022-05-19T18:05:24Z

src/glmtools.jl

+    return 2 * (xlogy(y, y / μ) - xlogy(y + 1, (y + 1) / (μ + 1)))
+end
 devresid(::InverseGaussian, y, μ::Real) = abs2(y - μ) / (y * abs2(μ))
 function devresid(d::NegativeBinomial, y, μ::Real)


Can you adapt this method to use the same pattern for consistency?

changed oftype(η, NaN) to convert(float(typeof(η)), NaN) wherever possible in inverselink functions since GLM.inverselink(GLM.IdentityLink(), 0) is throwing an error.

Also, the devresid function for Negative Binomial is defined as how it is defined for Geometric.

Thanks for your suggestion. The suggestion has been tested and committed. Co-authored-by: Milan Bouchet-Valat <nalimilan@club.fr>

… possible since `GLM.inverselink(GLM.IdentityLink(), 0)` is throwing an error.

nalimilan · 2022-05-20T08:01:59Z

src/glmtools.jl

 function inverselink(::InverseLink, η::Real)
    μ = inv(η)
-    return μ, -abs2(μ), oftype(μ, NaN)
+    return μ, -abs2(μ), convert(float(typeof(μ)), NaN)


These are likely not needed as μ = inv(η) will (almost?) always be a float. Though I guess that doesn't hurt much.

nalimilan · 2022-05-20T21:51:46Z

Thanks!

Mousum added 4 commits April 15, 2022 16:37

preserved the alphabetically order in list of the distributions, repl…

63e12a3

…ace `approx` function by `≈` in test cases.

replaced Negative with NegativeBinomial

ba54716

corrected a few things to pass the doc test

755e47f

nalimilan reviewed Apr 24, 2022

View reviewed changes

src/glmtools.jl Outdated Show resolved Hide resolved

test/runtests.jl Outdated Show resolved Hide resolved

docs/src/index.md Outdated Show resolved Hide resolved

mousum-github and others added 4 commits April 25, 2022 10:06

Update src/glmtools.jl

c05ee54

Co-authored-by: Milan Bouchet-Valat <nalimilan@club.fr>

Update test/runtests.jl

0754df0

Co-authored-by: Milan Bouchet-Valat <nalimilan@club.fr>

Update docs/src/index.md

01b3981

Co-authored-by: Milan Bouchet-Valat <nalimilan@club.fr>

added stderror in the test case

a1f9ef9

nalimilan requested review from dmbates and palday April 25, 2022 07:15

nalimilan approved these changes Apr 25, 2022

View reviewed changes

palday reviewed Apr 26, 2022

View reviewed changes

src/glmtools.jl Outdated Show resolved Hide resolved

palday approved these changes Apr 26, 2022

View reviewed changes

updated the devresid calculation

7ad3d3d

nalimilan reviewed May 19, 2022

View reviewed changes

mousum-github and others added 2 commits May 20, 2022 13:00

Update src/glmtools.jl

f5b9383

Thanks for your suggestion. The suggestion has been tested and committed. Co-authored-by: Milan Bouchet-Valat <nalimilan@club.fr>

changed oftype(η, NaN) to convert(float(typeof(η)), NaN) wherever…

633ed64

… possible since `GLM.inverselink(GLM.IdentityLink(), 0)` is throwing an error.

nalimilan reviewed May 20, 2022

View reviewed changes

nalimilan requested a review from palday May 20, 2022 08:02

palday approved these changes May 20, 2022

View reviewed changes

nalimilan merged commit 3a3a27c into JuliaStats:master May 20, 2022

Adding geometric distribution for GLM.jl #480

Adding geometric distribution for GLM.jl #480

Uh oh!

Conversation

ayushpatnaikgit commented Apr 22, 2022

Uh oh!

nalimilan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented Apr 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

palday left a comment

Choose a reason for hiding this comment

Uh oh!

nalimilan commented Apr 26, 2022

Uh oh!

ViralBShah commented May 19, 2022

Uh oh!

nalimilan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nalimilan commented May 20, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

codecov-commenter commented Apr 25, 2022 •

edited

Loading