dswah / pyGAM

[HELP REQUESTED] Generalized Additive Models in Python
https://pygam.readthedocs.io
Apache License 2.0
862 stars 159 forks source link

constraint doesn't work for s() when there is tensor term te() in the model #224

Closed jzang18 closed 5 years ago

jzang18 commented 5 years ago

using the "chicago example"

from pygam import PoissonGAM, s, te from pygam.datasets import chicago X, y = chicago(return_X_y=True) gam = PoissonGAM(s(0, n_splines=200) + te(3, 1) + s(2)).fit(X, y) gam_test = PoissonGAM(s(0, constraints='monotonic_inc') + te(3, 1) + s(2)).fit(X, y)


TypeError Traceback (most recent call last)

in () 6 gam3 = PoissonGAM(s(0, n_splines=200) + te(3, 1) + s(2)).fit(X, y) 7 ----> 8 gam_test = PoissonGAM(s(0, constraints='monotonic_inc') + te(3, 1) + s(2)).fit(X, y) ... anaconda/lib/python2.7/site-packages/pygam/terms.py in build_constraints(self, coef, constraint_lam, constraint_l2) 363 if constraint is None: 364 constraint = 'none' --> 365 if constraint in CONSTRAINTS: 366 constraint = CONSTRAINTS[constraint] 367 TypeError: unhashable type: 'list'
dswah commented 5 years ago

@jzang18 ahh, right. this is because TensorTerms dont have their own build_constraints() method.

thank you for finding and documenting all of these bugs so far.

jzang18 commented 5 years ago

is it possible to add a dummy (none) constraint method for TensorTerms and fix the bug? We probably don't need constraint for TensorTerms (interactions). You can see I am using this lib:). Really hope this library will be as good as some of the R gam libraries some time soon.

dswah commented 5 years ago

@jzang18 i am very glad you are trying out the library :)

However, i think we should allow tensor interactions to have constraints on the marginal terms. i believe leaving them out would be incomplete.

also, the PR for this fix is ready.

jzang18 commented 5 years ago

how to get this fix? -- I am new to git, I usually just use git clone, but it seems not getting this fix. thanks.

dswah commented 5 years ago

@jzang18 just published it to pypi. you should be able to do:
pip install -U pygam

please let me know how it works!

jzang18 commented 5 years ago

it works fine now. However, there might be some minor bug which causes my python session die repeatedly when I skip constraints in some terms. gam3x = PoissonGAM(s(0, constraints='monotonic_inc') + te(3, 1) + s(2)).fit(X, y)

The problem can be fixed by explicitly using constraints='none' in other terms the FIRST TIME. And it will be fine to skip those none constraints later on. Seems like some kind of state bug.

Didn't have time to try to producing it.

thanks for the quick fix!

dswah commented 5 years ago

@jzang18 that's weird. can you tell me more about your system? is this still python 2.7, numpy 1.11?

i couldnt reproduce that error...

please try to reproduce it, and send me some more clues. thanks.

-dani