Resolve "Implement graph learning" - [merged]

[lucaslanek]

In GitLab by @julioRodino on May 26, 2024, 16:22

Merges 3-implement-graph-learning -> master

Added function to learn weight matrix based on a graph signal. Added a test fucntion to test the correct functioning of the toolbox. Added an example in the examples folder.

The example learns a graph based on the coordenates (2D) of the nodes. This could be done with a graph signal but it's set this way since it follows from the example in GSP (https://epfl-lts2.github.io/gspbox-html/doc/demos/gsp_demo_learn_graph_large.html).

Closes #3

[lucaslanek]

In GitLab by @julioRodino on May 26, 2024, 16:22

requested review from @aweinstein

[lucaslanek]

In GitLab by @aweinstein on May 27, 2024, 09:52

Commented on 3f244aaa6841a95abedd2a1636d0043ad224cf0b

This is not the right file to place the graph learning code. It has code to solve "a problem where only some values of a graph signal are known, and the others shall be inferred." I suggest starting a new file named graph_learning.

[lucaslanek]

In GitLab by @aweinstein on May 27, 2024, 09:52

Commented on pygsp/learning.py line 401

Is there any reason not to use "Kalofolias, Vassilis. "How to learn a graph from smooth signals." Artificial intelligence and statistics. PMLR, 2016." instead of the ArXiv version?

[lucaslanek]

In GitLab by @aweinstein on May 27, 2024, 09:52

Commented on pygsp/learning.py line 374

Add space after comma (PEP 8).

[lucaslanek]

In GitLab by @aweinstein on May 27, 2024, 09:52

The code does not comply with PEP8. Please use a linter to find the non-compliant code.

[lucaslanek]

In GitLab by @aweinstein on May 27, 2024, 09:52

Commented on pygsp/learning.py line 450

Add an else clause where you check that w_0 is a vector and throw an exception otherwise.

[lucaslanek]

In GitLab by @aweinstein on May 27, 2024, 09:52

Commented on pygsp/learning.py line 451

Use np.zeros_like(Z) instead of np.zeros(Z.shape).

[lucaslanek]

In GitLab by @aweinstein on May 27, 2024, 09:52

Commented on pygsp/learning.py line 397

Change W to w.

[lucaslanek]

In GitLab by @aweinstein on May 27, 2024, 09:52

Commented on pygsp/utils.py line 366

Remove the extra double quote (I think your editor has something that adds the extra double quote).

[lucaslanek]

In GitLab by @aweinstein on May 27, 2024, 09:52

Commented on pygsp/utils.py line 371

Remove the empty line.

[lucaslanek]

In GitLab by @aweinstein on May 27, 2024, 09:52

requested changes

[lucaslanek]

In GitLab by @julioRodino on May 27, 2024, 10:29

Commented on 3f244aaa6841a95abedd2a1636d0043ad224cf0b

changed this file in version 2 of the diff

[lucaslanek]

In GitLab by @julioRodino on May 27, 2024, 10:29

Commented on pygsp/learning.py line 374

changed this line in version 2 of the diff

[lucaslanek]

In GitLab by @julioRodino on May 27, 2024, 10:29

Commented on pygsp/utils.py line 366

changed this line in version 2 of the diff

[lucaslanek]

In GitLab by @julioRodino on May 27, 2024, 10:29

added 1 commit

• a80becad - Fixed whitespace trailing, spaces after comas and formating to conform with PEP8 standards

Compare with previous version

[lucaslanek]

In GitLab by @julioRodino on May 27, 2024, 10:41

Commented on pygsp/learning.py line 397

Changing W to w could lead to confusion since the returned array is a 2d symmetric matrix, the square-form of the w vector. Should I change it anyways? or would you prefer we return the adjacency matrix in vector-form which corresponds to w? Now the function returns the matrix because it is ready to input into a graph structure of plot function.

[lucaslanek]

In GitLab by @julioRodino on May 27, 2024, 14:38

Commented on pygsp/learning.py line 401

changed this line in version 3 of the diff

[lucaslanek]

In GitLab by @julioRodino on May 27, 2024, 14:38

Commented on pygsp/learning.py line 450

changed this line in version 3 of the diff

[lucaslanek]

In GitLab by @julioRodino on May 27, 2024, 14:38

Commented on pygsp/learning.py line 451

changed this line in version 3 of the diff

[lucaslanek]

In GitLab by @julioRodino on May 27, 2024, 14:38

Commented on pygsp/learning.py line 397

changed this line in version 3 of the diff

[lucaslanek]

In GitLab by @julioRodino on May 27, 2024, 14:38

Commented on pygsp/utils.py line 371

changed this line in version 3 of the diff

[lucaslanek]

In GitLab by @julioRodino on May 27, 2024, 14:38

added 2 commits

• 81fdb586 - Fixed referece in documentation. Using zeros_like instead of zeroes....
• 00c0b367 - Added exeption in case w_0 is not a 2d array or 'zeros'

Compare with previous version

[lucaslanek]

In GitLab by @julioRodino on May 27, 2024, 14:45

added 1 commit

• 02ac9c9c - change exeption to TypeError

Compare with previous version

[lucaslanek]

In GitLab by @julioRodino on May 27, 2024, 14:53

added 3 commits

• 9e0f8c74 - fixed line too long
• b795c8d8 - change import in example
• 4d54ecf5 - fixed documentation. Docstring modifications and number of iterations.

Compare with previous version

[lucaslanek]

In GitLab by @julioRodino on May 27, 2024, 15:00

Commented on 3f244aaa6841a95abedd2a1636d0043ad224cf0b

File changed. Now there is a submodule called graph_learning

[lucaslanek]

In GitLab by @julioRodino on May 27, 2024, 15:01

Commented on pygsp/learning.py line 401

No. Changed to:

Vassilis Kalofolias Proceedings of the 19th International Conference on Artificial Intelligence and Statistics, PMLR 51:920-929, 2016.

[lucaslanek]

In GitLab by @julioRodino on May 27, 2024, 15:01

Commented on pygsp/learning.py line 374

Fixed

[lucaslanek]

In GitLab by @julioRodino on May 27, 2024, 15:05

Fixed in terms of format and linting.

Variable names do not conform to naming styles suggested in PEP8, though, this is not particular to this code. I think is preferable to have variable names equivalent to the names assigned in the paper.

[lucaslanek]

In GitLab by @julioRodino on May 27, 2024, 15:06

Commented on pygsp/learning.py line 450

added an TypeError if w_0 is not equal to 'zeros' or a 2d-numpy array.

[lucaslanek]

In GitLab by @julioRodino on May 27, 2024, 15:06

Commented on pygsp/learning.py line 451

fixed

[lucaslanek]

In GitLab by @julioRodino on May 27, 2024, 15:07

Commented on pygsp/utils.py line 366

fixed

[lucaslanek]

In GitLab by @julioRodino on May 27, 2024, 15:07

Commented on pygsp/utils.py line 371

fixed

[lucaslanek]

In GitLab by @julioRodino on May 27, 2024, 15:07

requested review from @aweinstein

[lucaslanek]

In GitLab by @aweinstein on May 28, 2024, 05:18

Commented on pygsp/learning.py line 397

You're right.

[lucaslanek]

In GitLab by @aweinstein on May 28, 2024, 05:22

OK with variable names. I think there is a way to tell the linter to skip some tests. We can check that later. Ideally, the whole project should go through PEP8 as a step of the CI.

[lucaslanek]

In GitLab by @aweinstein on May 28, 2024, 05:44

added 3 commits

• cab10221 - Change parameter to float values, to avoid type warnings.
• 8d685466 - Remove extra double quote.
• 06359493 - Change lines too long to less than 80 character.

Compare with previous version

[lucaslanek]

In GitLab by @aweinstein on May 28, 2024, 05:58

added 2 commits

• d7a51a86 - Add space between the operators.
• 3217dbcb - Use double quotes in string to avoid escaping the single quote.

Compare with previous version

[lucaslanek]

In GitLab by @aweinstein on May 28, 2024, 05:59

resolved all threads

[lucaslanek]

In GitLab by @aweinstein on May 28, 2024, 06:01

approved this merge request

[lucaslanek]

In GitLab by @aweinstein on May 28, 2024, 06:04

mentioned in commit cddc612cb17958bde6e23a6aa07b183ad518c7bc