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.

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

Issue with class Gamma #188

Closed
anthonybritto opened this issue Oct 6, 2023 · 2 comments
Closed

Issue with class Gamma #188

anthonybritto opened this issue Oct 6, 2023 · 2 comments

Comments

@anthonybritto
Copy link

anthonybritto commented Oct 6, 2023

I'm trying to construct the following model:

mod = GeneralizedLinearEstimator(Gamma(), skg.penalties.L1(alpha=0.5))

However, when I run mod.fit(exog, endog) I am met with

AttributeError: 'Gamma' object has no attribute 'intercept_update_step'

Looking at the documentation, I wonder if this is due a simple typo: I see that Gamma has the method intercept_update_self defined; should this be intercept_update_step instead?

Is there a possible workaround for now? I naively tried to define

g = Gamma()
g.intercept_update_step = g.intercept_update_self

but the error persists.

@mathurinm
Copy link
Collaborator

mathurinm commented Oct 6, 2023

Hi @anthonybritto, thanks for raising the issue.
it is indeed a typo, we will fix it. But the issue is actually something else: since you're not specifying a solver, skglm will use AndersonCD, which does not work for the Gamma datafit.

Instead you should use the ProxNewton solver, as follows:

solver = ProxNewton(verbose=2)
mod = GeneralizedLinearEstimator(Gamma(), skg.penalties.L1(alpha=0.5), solver=solver)

feel free to reopen if it does not fix your issue.

EDIT: you'll also need to disable intercept fitting for a while, as the current implementation is probably not correct.
EDIT: in fact as pointed out by @Badr-MOUFAD, intercept fitting should be correct for ProxNewton

@mathurinm
Copy link
Collaborator

mathurinm commented Oct 6, 2023

You'll need to checkout the branch #189 that fixes the issue, before we merge it into main.

EDIT: in fact it's currently implemented correctly in main, using a different method than intercept_update_step

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants