yngvem / group-lasso

Group Lasso implementation following the scikit-learn API
MIT License
105 stars 32 forks source link

Group LASSO vs. sparse group LASSO #24

Closed johnabel closed 3 years ago

johnabel commented 3 years ago

Hey @yngvem , it wasn't clear to me if this should be implementing the group LASSO method or the sparse group LASSO method. Based on the README, I think it is sparse group LASSO. However, lines 207-211 in src/group_lasso/_group_lasso.py state:

regulariser = 0
coef_ = _split_intercept(w)[1]
for group, reg in zip(self.groups_, self.group_reg_vector_):
    regulariser += reg * la.norm(coef_[group])
regulariser += self.l1_reg * la.norm(coef_.ravel(), 1)

and I therefore think this implements the standard group LASSO with since reg * la.norm(coef_[group]) is the L2 norm. It would, of course, also be simple to add a kwarg to let a user choose between group LASSO and sparse group LASSO.

One further detail: in BaseGroupLasso the kwarg l1_reg is set to 0.00 but the docstring states it is 0.05.

Submitting a PR to (1) implement the L1 norm for sparse group LASSO and (2) fix the default l1_reg.

mathurinm commented 3 years ago

@johnabel In the line you quoted you have regulariser += self.l1_reg * la.norm(coef_.ravel(), 1) , i.e. after computing the sum of the norms of the groups, you add the L1 norm of coef_, so it is a SparseGroupLasso indeed, no ?

johnabel commented 3 years ago

My mistake--I misread which norms were L1/L2 in the code. Thanks!