ENH Add support for GLasso and Adaptive (reweighted) GLasso #280

Perceptronium · 2025-02-10T13:17:51Z

Context of the PR

Add Graphical Lasso and Adaptive Graphical Lasso estimators

Contributions of the PR

A new GraphicalLasso estimator that can:

Handle weighted regularization.
Use two different algorithms to solve the GLasso problem:
2.1. The "Banerjee" approach as in Banerjee et al., 2008 (the original GLasso algorithm)
2.2. The "Mazumder" approach as in Mazumder et al., 2012 (the P-GLasso algorithm)

A new AdaptiveGraphicalLasso estimator that solves the Reweighted Graphical Lasso problem, where the weights are updated following the procedure of Candès et al., 2008
Test functions for the two estimators
An illustrative example of the two estimators that plots their performance (in terms of NMSE and F1 score) as a function of the regularization hyperparameter.

…asso

Perceptronium · 2025-02-10T13:19:00Z

Hello @Badr-MOUFAD ,
Hope everything is going well for you ! Could you take a look at this when you have the time ?
Thanks a lot !

Badr-MOUFAD · 2025-02-10T13:30:51Z

Definitely! Thnx for the PR 🚀

mathurinm · 2025-02-14T08:03:51Z

@Badr-MOUFAD no need to review yet, we realized that a barebone gramcd solver works much better than anderson working set here, we're still in the benchmarking phase. We'll ping you when the code is ready! 🙏

…s sklearn

mathurinm · 2025-04-01T07:54:53Z

@Perceptronium the unit tests are failing:

you need to lint the code (run flake8 to see where the issues are ; customize VS code so that it shows the error directly in the editor)
you need to format the example according to what Sphinx expects, e.g. using a docstring at the top of the file: https://app.circleci.com/pipelines/github/scikit-learn-contrib/skglm/1711/workflows/f9170fae-79d7-4077-bdaa-d76d2b8f4e62/jobs/1609
check out how the existing examples are structured and follow this pattern

examples/plot_graphical_lasso.py

skglm/estimators.py

skglm/solvers/gram_cd.py

skglm/utils/data.py

…into graphical_lasso

…d tests file

mathurinm · 2025-04-11T07:26:59Z

skglm/covariance.py

+    elif strategy == "mcp":
+        gamma = 3.
+        Weights = np.zeros_like(Theta)
+        Weights[np.abs(Theta)


I find this indentation quite hard to read

mathurinm · 2025-04-11T07:27:16Z

skglm/covariance.py

+        return self
+
+
+def update_weights(Theta, alpha, strategy="log"):


This method should be private (start with an underscore)

mathurinm · 2025-04-11T07:27:53Z

skglm/covariance.py

+
+
+def update_weights(Theta, alpha, strategy="log"):
+    if strategy == "log":


Can you try to directly use penalty.derivative as is done in IterativeReweightedL1 ?

this one is changed now (see class above AdaptiveGraphicalLassoPenalty), ready for review and to be discussed with Can. I have made tests below which we can delete later (see results in screenshot attached)
To-Dos left: Verify if its correct, check if it works with all penalties
For now, both options yield the same results in the comparison

into perceptroniumglasso/graphical_lasso Merge Perceptronium/skglm's graphical_lasso: Sync with upstream for collaborative PR.#

mathurinm · 2025-04-15T12:34:54Z

@Perceptronium you can pull upstream main to fix the tests

…phical_lasso

…dos: review math, check if it works with all other penalties and clean up code to one version if everything is correct

into graphical_lasso

Badr-MOUFAD

great work, pin me when you are done for another review

Badr-MOUFAD · 2025-06-27T14:32:30Z

skglm/covariance.py

+
+
+class GraphicalLasso():
+    """A first-order BCD Graphical Lasso solver.


elaborate more on the docstring (attributes, refs, ...)
(for instance docs of AndersonCD)

Badr-MOUFAD · 2025-06-27T14:34:05Z

skglm/covariance.py

+from skglm.penalties.separable import L0_5
+
+
+class GraphicalLasso():


is there are reason why we don't inherent from sklearn estimator or at least BaseGraphicalLasso
?

that's will be a good way to stay consistent with sklearn api

Badr-MOUFAD · 2025-06-27T14:35:29Z

skglm/covariance.py

+            W = self.covariance_
+        else:
+            W = S.copy()
+            W *= 0.95


better be more explicit about why we scale W

As far as I understood we need it to get the inverse right after and thus W needs to be SPD. I changed the approach to not rely on hardcoded scaling, but instead computed the smallest eigenvalue of S, then if it was negative or too close to zero added a ridge to make all eigenvalues at least eps.
Let me know what you think.
@Perceptronium also check if there was any other reason for that scaling.

Badr-MOUFAD · 2025-06-27T14:36:36Z

skglm/covariance.py

+        return self
+
+
+class AdaptiveGraphicalLasso():


Better inherent a sklearn estimator here also

Badr-MOUFAD · 2025-06-27T14:39:09Z

skglm/covariance.py

+            glasso.weights = Weights
+            glasso.fit(S)
+
+            Theta_sym = (glasso.precision_ + glasso.precision_.T) / 2


is there a reason for enforcing the symmetry ? the G-Lasso should normally output symmetric matrices ?

was rather made as a guardrail in case there are small numerical errors and its slightly asymmetric, but I can remove it

Badr-MOUFAD · 2025-06-27T14:42:02Z

skglm/solvers/gram_cd.py

+
+
+@njit
+def barebones_cd_gram(H, q, x, alpha, weights, max_iter=100, tol=1e-4):


this function (almost) reimplement the routine in GramCD solver

skglm/skglm/solvers/gram_cd.py

Lines 148 to 167 in 4b2344c

def _gram_cd_epoch(scaled_gram, w, grad, penalty, greedy_cd):

all_features = np.arange(len(w))

for cd_iter in all_features:

# select feature j

if greedy_cd:

opt = penalty.subdiff_distance(w, grad, all_features)

j = np.argmax(opt)

else: # cyclic

j = cd_iter

# update w_j

old_w_j = w[j]

step = 1 / scaled_gram[j, j] # 1 / lipschitz_j

w[j] = penalty.prox_1d(old_w_j - step * grad[j], step, j)

# gradient update with Gram matrix

if w[j] != old_w_j:

grad += (w[j] - old_w_j) * scaled_gram[:, j]

return penalty.subdiff_distance(w, grad, all_features)

we better wrap it than re-implemented

Generally good suggestion to avoid code duplication. However, I am a bit cautious with doing changes there: (see comment from Mathurin: @#280 (comment) )
As I understood, Can and Mathurin made the separate barebones_cd_gram function for performance:

The graphical lasso inner loop is extremely performance-sensitive and is called many times

The general GramCD/_gram_cd_epoch routine is more flexible, but that flexibility adds overhead (penalty abstraction, selection logic, etc.).

The specialized function is Numba-compiled and hardcoded for L1/soft-thresholding, which probably makes it much faster for this use case.

If you think we can refactor to share more code without losing performance, I’m open to suggestions. I didnt manage to wrap it without getting into issues with the numba compilation.

@mathurinm @Perceptronium, it seems you did some benchmarking back in February on this, maybe you can help us out here what to do best?

I think it is doable without compromising performance (but, maybe I'm wrong)
I will try to push code to see

@Badr-MOUFAD yesI am the one who wrote this, when using our existing code we were too slow. I'd be happy if you managed to improve, but otherwise (with the addition of a comment to explain why we need this) I'm ok with this solution as it only concerns a small snippet

@Badr-MOUFAD I tried to make a fair comparison with the standard gram_cd and the epoch solver, but I couldn't figure out a way to make them faster and in the comparison, barebones is still the fastest. Let me know what you think. Just run the debug_compare_solvers script and you will see it.

(Note: For the comparison I had to let GramCD take gram as an input instead of the design matrix, so I added this. It was marked as a todo anyways, so please verify if I implemented it correctly.

@Badr-MOUFAD If you're OK with this, I'm in favor of incorporating these changes. If another version is faster, it can be replaced in a subsequent PR. OK?

Sorry, didn't have the time to push.
Go ahead, we can make a PR later if needed

What explains the time difference is probably the full pass over w when returning subdiff distance in GramCD:

skglm/skglm/solvers/gram_cd.py

Line 167 in 4b2344c

return penalty.subdiff_distance(w, grad, all_features)

…, remove symmetry enforcing

…CD solver

mathurinm · 2025-07-08T15:56:04Z

skglm/solvers/gram_cd.py

+            scaled_gram = X
+            scaled_Xty = y
+            n_features = X.shape[0]
+            scaled_y_norm2 = 0


this is not correct if we do not pass y, right?

…resolve merge conflict

…his PR

add support for graphical lasso and adaptive (reweighted) graphical l…

c199221

…asso

mathurinm marked this pull request as draft February 14, 2025 08:04

mathurinm changed the title ~~Add support for GLasso and Adaptive (reweighted) GLasso~~ WIP Add support for GLasso and Adaptive (reweighted) GLasso Feb 14, 2025

Perceptronium added 2 commits March 20, 2025 11:37

Update estimators to use barebones solver, make it at least as fast a…

5696d40

…s sklearn

add Reweighted GLasso regularization path example

3e3df79

mathurinm requested changes Apr 1, 2025

View reviewed changes

Perceptronium added 4 commits April 2, 2025 14:13

Merge branch 'main' of https://github.com/scikit-learn-contrib/skglm …

c9da9d4

…into graphical_lasso

fix issues in glasso reg path example

ca6960f

fix glasso solver issues, move estimator to own file, create dedicate…

4538002

…d tests file

remove snakecase function names in test functions

a4ea3fd

Perceptronium marked this pull request as ready for review April 3, 2025 08:03

mathurinm changed the title ~~WIP Add support for GLasso and Adaptive (reweighted) GLasso~~ ENH Add support for GLasso and Adaptive (reweighted) GLasso Apr 7, 2025

mathurinm reviewed Apr 11, 2025

View reviewed changes

floriankozikowski added 3 commits April 11, 2025 14:59

Merge branch 'graphical_lasso' of https://github.com/Perceptronium/skglm

ff9c2fe

into perceptroniumglasso/graphical_lasso Merge Perceptronium/skglm's graphical_lasso: Sync with upstream for collaborative PR.#

adjust weight updates, still need to test

3148357

trying out different update methods, no success so far

082c00d

mathurinm and others added 7 commits April 22, 2025 16:18

Merge branch 'main' of github.com:scikit-learn-contrib/skglm into gra…

7fc0f21

…phical_lasso

empty

a4e192f

add explicit handling of zeros in penalties.derivative, works now, to…

33ca52c

…dos: review math, check if it works with all other penalties and clean up code to one version if everything is correct

leave original name of old strategy based version, so tests dont fail

fc70009

fix minor name dependency

a927e50

ci trigger

b62990d

Merge remote-tracking branch 'upstream/main' into graphical_lasso

02f0fb5

floriankozikowski added 5 commits June 26, 2025 16:55

Merge branch 'graphical_lasso' of https://github.com/Perceptronium/skglm

264faed

into graphical_lasso

fix linters

16dbe4e

fix pytests

c50f5d3

clean up & fix adaptive method, update example, unit test accordingly

c9d1545

edit what's new, and api

3f833cd

mathurinm requested a review from Badr-MOUFAD June 27, 2025 14:11

Badr-MOUFAD reviewed Jun 27, 2025

View reviewed changes

floriankozikowski and others added 5 commits June 30, 2025 18:23

make sklearn compatible, add docstrings, improve init. of cov. matrix…

ad857f2

…, remove symmetry enforcing

fix doc example, make_dummy_covariance_data

653a431

adjust author block

b79859e

verify alternatives to barebones solver, allow gram as input for Gram…

08c1896

…CD solver

option to disable subdiff computation inn cd_gram

d7824c1

mathurinm reviewed Jul 8, 2025

View reviewed changes

floriankozikowski added 4 commits July 8, 2025 18:38

remove debugging, clean up, add math to doc

ce0938f

Merge remote-tracking branch 'upstream/main' into graphical_lasso to …

f6273bd

…resolve merge conflict

edit whats new after merge conflict

183adfa

revert gram file back to original as no changes needed in there for t…

e4333c7

…his PR

mathurinm approved these changes Jul 17, 2025

View reviewed changes

mathurinm merged commit 7e4802b into scikit-learn-contrib:main Jul 17, 2025
4 checks passed

		return self


		def update_weights(Theta, alpha, strategy="log"):



		def update_weights(Theta, alpha, strategy="log"):
		if strategy == "log":



		class GraphicalLasso():
		"""A first-order BCD Graphical Lasso solver.

		from skglm.penalties.separable import L0_5


		class GraphicalLasso():



		@njit
		def barebones_cd_gram(H, q, x, alpha, weights, max_iter=100, tol=1e-4):

	def _gram_cd_epoch(scaled_gram, w, grad, penalty, greedy_cd):
	all_features = np.arange(len(w))
	for cd_iter in all_features:
	# select feature j
	if greedy_cd:
	opt = penalty.subdiff_distance(w, grad, all_features)
	j = np.argmax(opt)
	else: # cyclic
	j = cd_iter

	# update w_j
	old_w_j = w[j]
	step = 1 / scaled_gram[j, j] # 1 / lipschitz_j
	w[j] = penalty.prox_1d(old_w_j - step * grad[j], step, j)

	# gradient update with Gram matrix
	if w[j] != old_w_j:
	grad += (w[j] - old_w_j) * scaled_gram[:, j]

	return penalty.subdiff_distance(w, grad, all_features)

ENH Add support for GLasso and Adaptive (reweighted) GLasso #280

ENH Add support for GLasso and Adaptive (reweighted) GLasso #280

Uh oh!

Conversation

Perceptronium commented Feb 10, 2025

Context of the PR

Contributions of the PR

Uh oh!

Perceptronium commented Feb 10, 2025

Uh oh!

Badr-MOUFAD commented Feb 10, 2025

Uh oh!

mathurinm commented Feb 14, 2025

Uh oh!

mathurinm commented Apr 1, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

floriankozikowski Apr 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mathurinm commented Apr 15, 2025

Uh oh!

Badr-MOUFAD left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

floriankozikowski Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

floriankozikowski Apr 22, 2025 •

edited

Loading

floriankozikowski Jul 7, 2025 •

edited

Loading