Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Pass distribution objects into GLM and GLMCV class #350

Open
pavanramkumar opened this issue Nov 4, 2019 · 1 comment · May be fixed by #390
Open

Pass distribution objects into GLM and GLMCV class #350

pavanramkumar opened this issue Nov 4, 2019 · 1 comment · May be fixed by #390
Labels
API enhancement
Projects
remote sprint august 2020

Comments

@pavanramkumar
Copy link
Collaborator

Motivation: as we expand the scope of models supported, it may be easier for contributors to add new distributions if we refactor the class to accept distribution-specific params and corresponding likelihood / loss / gradients / hessians as callables.

One strategy inspired by statsmodels proposed by @jasmainak in #274 is as follows:

distr = NegativeBinomial(n_failure)
GLM(distr=distr)

class MyDistribution(BaseDistribution):
    def loglikelihood(self):
        pass
    def loglik_gradient(self):
       pass
   def loglik_hessian(self):
       pass

There are also alternatives such as the scikit-learn approach which is less work for the low touch user who only needs to call fit and predict on a model object without having to construct a distr object:

model = PoissonGLM(base_param1, base_param2, poisson_param1, poisson_param2)

class PoissonGLM(GLM):
    def loglikelihood(self):
        pass
    def loglik_gradient(self):
       pass
   def loglik_hessian(self):
       pass
@jasmainak
Copy link
Member

I wouldn't change the class signature too much any more. We made one big backwards incompatible change with this release (GLM and GLMCV). Now we should try to keep the API a little stable. Not sure I understand the second approach fully. In the first approach, you would not change anything in the GLM object except allow also objects that subclass from BaseDistribution. Users don't have to create them as you can just provide them in a module.

from pyglmnet.distributions import NegativeBinomial

distr = NegativeBinomial(n_failure)

then inside the GLM class you'd say:

if distr == 'negative_binomial':
    self.distr = NegativeBinomial()
elif distr == 'poisson':
   self.distr = Poisson()

etc. The subclassing simply ensures that all the distributions follow the same pattern with the same methods that need to be overridden.

@jasmainak jasmainak removed this from To do in remote sprint august 2020 Aug 15, 2020
@jasmainak jasmainak added this to To do in remote sprint august 2020 via automation Aug 15, 2020
@pavanramkumar pavanramkumar linked a pull request Aug 16, 2020 that will close this issue
Open
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
API enhancement
Projects
remote sprint august 2020
  
To do
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[WIP] distribution classes pavanramkumar/pyglmnet
2 participants
@pavanramkumar @jasmainak