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

Fix Segmentation Fault and ZeroDivisionError in Group Lasso #292

Merged
merged 17 commits into from
Apr 8, 2024
Merged

Fix Segmentation Fault and ZeroDivisionError in Group Lasso #292

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
dd40914
fix index & divide by zero
Badr-MOUFAD Mar 29, 2024
cd77bb6
provide kwarg to _preprocess_data
Badr-MOUFAD Mar 29, 2024
afaf973
fit_intercept as bool
Badr-MOUFAD Mar 29, 2024
629cee7
CI trigger
Badr-MOUFAD Mar 29, 2024
028ec44
fix index group
Badr-MOUFAD Mar 30, 2024
b988f1a
debug circle ci
Badr-MOUFAD Mar 30, 2024
169be60
print pwd
Badr-MOUFAD Mar 30, 2024
a87fbf8
print pwd
Badr-MOUFAD Mar 30, 2024
2e89487
content of dir
Badr-MOUFAD Mar 30, 2024
de6133c
MNT - remove requirements files (#285)
Badr-MOUFAD Apr 19, 2023
265d2c9
MNT cosmit readme (#287)
mathurinm Jul 21, 2023
71f8dc8
CI trigger
Badr-MOUFAD Mar 29, 2024
a6de333
debug circle ci
Badr-MOUFAD Mar 30, 2024
69372b8
print pwd
Badr-MOUFAD Mar 30, 2024
32309a1
content of dir
Badr-MOUFAD Mar 30, 2024
c4d6a43
rm commands circle-ci
Badr-MOUFAD Mar 30, 2024
138bd97
Merge branch 'main' of github.com:mathurinm/celer into fix-grp
mathurinm Apr 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion celer/dropin_sklearn.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -646,7 +646,7 @@ def fit(self, X, y):
% (n_samples, y.shape[0]))

X, y, X_offset, y_offset, X_scale = _preprocess_data(
X, y, self.fit_intercept, copy=False)
X, y, fit_intercept=self.fit_intercept, copy=False)

if not self.warm_start or not hasattr(self, "coef_"):
self.coef_ = None
Expand Down
9 changes: 5 additions & 4 deletions celer/group_fast.pyx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ cpdef floating dnorm_grp(

else: # scaling only with features in C
for g_idx in range(ws_size):
if weights[g] == INFINITY:
if weights[g_idx] == INFINITY:
continue

g = C[g_idx]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are you sure that it's not g (defined L95, so def should be moved above) which should be used here? weights has size n_groups, not ws_size

Copy link
Collaborator Author

@Badr-MOUFAD Badr-MOUFAD Mar 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes you are right,
We can insert line 95 right after Line 91 and keep the rest as it is

Expand Down Expand Up @@ -418,8 +418,11 @@ cpdef celer_grp(
&inc) / lc_groups[g]
norm_wg += w[j] ** 2
norm_wg = sqrt(norm_wg)
bst_scal = max(0.,
if norm_wg != 0.:
bst_scal = max(0.,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm perhaps lacking context about the code, namely I ignore the purpose of bst_scal.

I just followed commun sense to handle the case of norm w being zero.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bst_scal is for BockSofthThresholding scaling : the formula for the BST of wg at level lambda is:
wg * max(0, 1 - lambda / norm(wg)) aka 0 if norm(wg) < lambda, and (1 - lambda/norm(wg)) wg otherwise

I'm curious, how do you end up with a vanishing wg after a gradient step ? This should not happen with probability 1, I'm guessing a 0 group X_g ?

1. - alpha * weights[g] / lc_groups[g] * n_samples / norm_wg)
else:
bst_scal = 0.

for k in range(grp_ptr[g + 1] - grp_ptr[g]):
j = grp_indices[grp_ptr[g] + k]
Expand Down Expand Up @@ -448,5 +451,3 @@ cpdef celer_grp(
'Fitting data with very small alpha causes precision issues.',
ConvergenceWarning)
return np.asarray(w), np.asarray(theta), np.asarray(gaps[:t + 1])


2 changes: 1 addition & 1 deletion celer/tests/test_mtl.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@


@pytest.mark.parametrize("sparse_X, fit_intercept",
itertools.product([0, 1], [0, 1]))
itertools.product([0, 1], [False, True]))
def test_GroupLasso_Lasso_equivalence(sparse_X, fit_intercept):
"""Check that GroupLasso with groups of size 1 gives Lasso."""
n_features = 1000
Expand Down
Loading