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

Bug: Negative eigenvalues from normalized_hypergraph_laplacian #647

Closed
kaiser-dan opened this issue Jan 24, 2025 · 2 comments · Fixed by #648
Closed

Bug: Negative eigenvalues from normalized_hypergraph_laplacian #647

kaiser-dan opened this issue Jan 24, 2025 · 2 comments · Fixed by #648
Labels
bug Something isn't working

Comments

@kaiser-dan
Copy link
Contributor

Summary

The normalized hypergraph Laplacian of a hypergraph with non-uniformly sized edges is capable of returning negative eigenvalues.

Example

Below is a (minimal?) working example

import xgi
import numpy as np
from scipy.linalg import eigh

E = [
    {1, 2, 3},
    {1, 4, 5},
    {1, 6, 7, 8},
    {4, 9, 10, 11, 12},
    {1, 13, 14, 15, 16},
    {4, 17, 18}
]
H = xgi.Hypergraph(E)

L = xgi.normalized_hypergraph_laplacian(H, sparse=False)
evals, evecs = eigh(L)

assert np.all(evals >= 0)

Suspected problem

The referenced paper [1] in the implementation states that the Laplacian should have a trivial eigenvalue, eigenvector pair with eigenvalue 0 and eigenvector $(\sqrt{k_i})_i$. Since the Laplacian is real symmetric and positive semi-definite, it should also have non-negative real eigenvalues so I believe that eigenvalues are impossible.

Looking through the source implementation, I suspect the problem is that the current implementation utilizes a simplification of the Laplacian that is only valid in 2-uniform hypergraphs as derived in [1].

Suggested fix

I have been able to circumvent this error by relying on the definition directly in [1], i.e.,
$$\mathcal{L} = I - D_v^{-\frac{1}{2}}HD_e^{-1}H^{T}D_v^{-\frac{1}{2}}$$ (for unweighted hypergraphs).

I am happy to create a pull request!

Also, this is my first time submitting an issue to a public repo - sorry if I have done anything wrong with it!


[1] “Learning with Hypergraphs: Clustering, Classification, and Embedding” by Dengyong Zhou, Jiayuan Huang, Bernhard Schölkopf Advances in Neural Information Processing Systems (2006)

@maximelucas
Copy link
Collaborator

maximelucas commented Jan 24, 2025

Hi @kaiser-dan, thanks for raising this issue, you exposed things very clearly!

After a quick check of the ref, I agree with you, it looks like we should've used the general formula.

For future reference, the original ref even works for weighted hypergraphs by adding a weight matrix $W$, where $\Delta$ is the Laplacian (top of page 5):

Image

Feel free to create a pull request with your current solution, thanks so much!

@maximelucas maximelucas added the bug Something isn't working label Jan 24, 2025
@kaiser-dan
Copy link
Contributor Author

Okie done, I have created a pull request #648

kaiser-dan added a commit to kaiser-dan/xgi that referenced this issue Jan 29, 2025
Add the minimum(?) working example of issue xgi-org#647 as new `test_`
function.
Tidied `test_normalized_hypergraph_laplacian` and added L2 and L3
comparison.
nwlandry pushed a commit that referenced this issue Jan 30, 2025
* fix: rewrite `normalized_hypergraph_laplacian`

Fixes the implementation of `normalized_hypergraph_laplacian` to prevent
negative eigenvalues. Rewrites core matrix calculations in full
definition.

* test: add unit tests for updated laplacian

Adds a proprty test for eigenvalue sign.
Adds error tests for new `weights` variable.

* doc: update `normalized_hypergraph_laplacian` doc

Updated 'Raises' portion of function docstring to include type and
length error catches on `weights` parameter.

* test: added issue #657 m.w.e. as test

* fix(test): fix typo in unit test

* Update xgi/linalg/laplacian_matrix.py

Implement suggestion - Edge weight matrix

Co-authored-by: Maxime Lucas <maximelucas@users.noreply.github.com>

* test: add sqrt(d) eigenvector, true_L tests

* feat: update weighted argument for laplacian

* doc: update normalized_hypergraph_laplacian args

* Update xgi/linalg/laplacian_matrix.py

Fix docstring typo

Co-authored-by: Maxime Lucas <maximelucas@users.noreply.github.com>

* Update xgi/linalg/laplacian_matrix.py

Co-authored-by: Maxime Lucas <maximelucas@users.noreply.github.com>

* Update xgi/linalg/laplacian_matrix.py

Co-authored-by: Maxime Lucas <maximelucas@users.noreply.github.com>

* Update xgi/linalg/laplacian_matrix.py

Co-authored-by: Maxime Lucas <maximelucas@users.noreply.github.com>

* Update xgi/linalg/laplacian_matrix.py

Co-authored-by: Maxime Lucas <maximelucas@users.noreply.github.com>

* Update xgi/linalg/laplacian_matrix.py

Co-authored-by: Maxime Lucas <maximelucas@users.noreply.github.com>

* Update xgi/linalg/laplacian_matrix.py

Co-authored-by: Maxime Lucas <maximelucas@users.noreply.github.com>

* Update xgi/linalg/laplacian_matrix.py

Co-authored-by: Maxime Lucas <maximelucas@users.noreply.github.com>

* Update xgi/linalg/laplacian_matrix.py

Co-authored-by: Maxime Lucas <maximelucas@users.noreply.github.com>

* Update xgi/linalg/laplacian_matrix.py

Implement suggestion - Edge weight matrix

Co-authored-by: Maxime Lucas <maximelucas@users.noreply.github.com>

* feat: update weighted argument for laplacian

* doc: update normalized_hypergraph_laplacian args

* Update xgi/linalg/laplacian_matrix.py

Fix docstring typo

Co-authored-by: Maxime Lucas <maximelucas@users.noreply.github.com>

Update xgi/linalg/laplacian_matrix.py

Co-authored-by: Maxime Lucas <maximelucas@users.noreply.github.com>

Update xgi/linalg/laplacian_matrix.py

Co-authored-by: Maxime Lucas <maximelucas@users.noreply.github.com>

Update xgi/linalg/laplacian_matrix.py

Co-authored-by: Maxime Lucas <maximelucas@users.noreply.github.com>

Update xgi/linalg/laplacian_matrix.py

Co-authored-by: Maxime Lucas <maximelucas@users.noreply.github.com>

Update xgi/linalg/laplacian_matrix.py

Co-authored-by: Maxime Lucas <maximelucas@users.noreply.github.com>

Update xgi/linalg/laplacian_matrix.py

Co-authored-by: Maxime Lucas <maximelucas@users.noreply.github.com>

Update xgi/linalg/laplacian_matrix.py

Co-authored-by: Maxime Lucas <maximelucas@users.noreply.github.com>

Update xgi/linalg/laplacian_matrix.py

Co-authored-by: Maxime Lucas <maximelucas@users.noreply.github.com>

* test: separated #647 m.w.e. into new test

Add the minimum(?) working example of issue #647 as new `test_`
function.
Tidied `test_normalized_hypergraph_laplacian` and added L2 and L3
comparison.

* feat: update scipy sparse array to modern use

* chore: update diags_array scipy use in 'laplacian'

* Update xgi/linalg/laplacian_matrix.py

Co-authored-by: Maxime Lucas <maximelucas@users.noreply.github.com>

---------

Co-authored-by: Maxime Lucas <maximelucas@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants