search

epfl-lts2 / gspbox

Graph Signal Processing in Matlab
https://epfl-lts2.github.io/gspbox-html/
GNU General Public License v3.0
136 stars 56 forks source link

Graph multiresolution: largest_eigenvector #3

Open mys007 opened 7 years ago

mys007 commented 7 years ago

I think the behavior on L141 in gsp_graph_multiresolution.m is wrong in the case when largest_eigenvector(1)==0, which now leads to filling largest_eigenvector with 0s as sign(0)=0. I suggest L141 being executed only if largest_eigenvector(1)~=0. Also, as eig works up to eps precision, one should IMHO have nonnegative_logicals=(largest_eigenvector >= -eps); instead of nonnegative_logicals=(largest_eigenvector >= 0);, as the current may lead to problems with graphs with multiple connected components, which are then numerically not properly identified to be kept, which may lead to NaNs in Kron reduction.

mdeff commented 7 years ago

Thanks for reporting the issue. Note however that this toolbox is not developed nor maintained anymore. Please consider using the pygsp instead.

mys007 commented 7 years ago

Thanks for your reply! When/if I come back to Kron reduction again, I will definitely use the new Python toolbox (looks very nice, great work) and I will check if my two issues are still relevant there.

mdeff commented 7 years ago

Thanks :) I think they'll be relevant, though I did not touch the graph multiresolution code (it was initially translated from matlab by interns). Please submit a PR if you come across them again.