Add a graph learning module

[nperraud]

Add a small module to learn the graph from the signals.

From the user side, he can simply use the class LearnGraph.

[nperraud]

@kalofolb It would be great if you have a look at the code and the doc. Ideally you could make a small tutorial, similarly to https://github.com/epfl-lts2/pygsp/tree/master/doc/tutorials @mdeff The code is based on this branch https://github.com/epfl-lts2/pygsp/tree/naspert-nn_refactor It should be merged after the other one.

[coveralls]

Coverage increased (+0.7%) to 88.409% when pulling 27ed937a3eefe6138d20de93360ae2d41d8cd258 on learngraphnew into 49ff67918d7d7b723cfa58d1bee017809627bd6b on master.

[mdeff]

@nperraud, what is the status of this PR? Did @kalofolb check?

Some comments:

• It would be great to have an example or tutorial about this.
• Can we find a more precise and less generic name than pygsp.graphs.LearnGraph? (Also, don't use a verb for class names.)
• What is nn.py and nn-test.py in the root directory? That surely doesn't belong there.
• I will move pygsp/_nearest_neighbor.py into the pygsp.graphs package in #43.
• Don't add

  if __name__ == '__main__':
    unittest.main()

  in test modules. You can partially run the test suite, at various degrees of granularity, as follows:

  $ python -m unittest pygsp.tests.test_docstrings.suite_reference
  $ python -m unittest pygsp.tests.test_graphs.TestImportExport
  $ python -m unittest pygsp.tests.test_graphs.TestImportExport.test_save_load

[nperraud]

Did not hear from @kalofolb , I should probably do it myself.

• I will do a tutorial, but this should not prevent you for merging this PR
• I removed the nn.py and nn-test.py files.
• I removed

  if __name__ == '__main__':
     unittest.main()

• I am not sure for the name... We could use LearnedGraph? But it is still a verb... Any idea?

[mdeff]

Thanks! Yeah, we can do the tutorial in a separate PR.

My problem with the name is that LearnGraph is too generic. There are many ways to infer a graph from a set of features, like NearestNeighborsGraph. In my opinion, the name should give some information about how it does it, again like NearestNeighborsGraph.

[nperraud]

Here the method is based on smoothness, but I am not sure that this does help for the name. Another way might be that the classe LearnedGraph would take an argument to switch between learning methods. I would not consider a NNGraph as a learned graph.

[mdeff]

Better to keep one class per method (the separation is clear and the parameters would be messy otherwise).

Well, NNGraph also builds a graph from a set of features. The difference is in the assumptions the methods make about the features.

What about something like graphs.LearnFromSmoothSignals? Not sure about learn or infer, nor signals or features or data. (Alternative methods would go as graphs.Learn*.)

BTW, graphs.NNGraph should probably be renamed graphs.NearestNeighbors. Will do in #43.

[nperraud]

I think it should be LearnedFromSmoothSignals following your idea to not put verbs in class. I think we should keep the word Learned. I am OK with it, but it is not great... Shall I rename the file LearnGraph.py to LearnedGraph.py in the spirit that we put all future LearnedGraph there?

[mdeff]

Let's think a bit more about the name. If we'll group them, you can name the module graphs/learned.py.

[kalofolias]

Did not hear from @kalofolb , I should probably do it myself.

Hi guys, I will do the tutorial! Using new account, didn't take check the old one and missed the conversation.

About the name: if we call it "LearnFromSmoothSignals" it doesn't separate between my method and the one of Dong etal. The separation comes from the regularisation of the degrees vector, so there could be a parameter "method" or "model" with choices between "l2_degrees" or "log_degrees" or similar. But as @mdeff said, then we have different sets of parameters... So how do we name mine versus he one of Dong etal? LearnFromSmoothSignalsLog vs LearnFromSmoothSignalsL2?

[mdeff]

Good to hear from you @kalofolias! Thanks for the tutorial.

Do you like LearnedFromSmoothSignals as a name? Any other idea? Are there settings where we'd prefer Dong et al. over this one? If not, we might as well not wonder about the difference. Will it be implemented? If not, we can think about differentiating the methods when it'll be.

[kalofolias]

Yeah, I think it's an ok name... I can't think of a better one myself. The settings where Dong et al performs better are spotty, but I might implement it for completeness and comparison reasons later.

[nperraud]

I have renamed the graph... But the import is a bit dirty in the __init__.py. To make it clean, I would need to make a subfolder I think. @mdeff I think that you wanted to eventually remove the folder for the nngraphs. Let me know what you want me to do. Also, do you want me to make the code more pep8?

[kalofolias]

Hi guys, First tutorial is ready! I need permissions with this account to push. I also made some improvements to the code by @nperraud and fixed a bug (the distances in matrix Z have to be squared!).

[nperraud]

I cannot give you access... @mdeff , does he really need permission? If yes, can you add him?

[mdeff]

Thanks for your patience. @kalofolias, you should have received an invitation. Upon accepting, you should be able to push your changes to this branch. Thanks for the tutorial!

[nperraud]

Just found a small bug. @kalofolias , please pull

[nperraud]

oh actually you found it as well sorry

[mdeff]

Is @kalofolias actively using this? Can we push his improvements and tutorial while we have it in our mind? Thanks :)

[mdeff]

@nperraud: why wasn't this bug caught by the unit tests? Can we add some to avoid further issues?