MIPLabCH / nigsp

NiGSP: a module for graph signal processing on neuroimaging data
https://nigsp.readthedocs.io/en/latest/
Apache License 2.0
18 stars 11 forks source link

Fix computation of degree matrix (from normal matrix to its adjacency) #67

Closed smoia closed 1 year ago

smoia commented 1 year ago

Closes #

Fix an error for which degree matrices were computed from the original input matrix, not the adjacency matrix. This is not a problem if the input matrix and the adjacency are the same, but if by any reason they are not, the degree matrix would be wrong.

Proposed Changes

Change Type

Checklist before review

codecov[bot] commented 1 year ago

Codecov Report

Merging #67 (2f841be) into master (059532f) will increase coverage by 0.31%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #67      +/-   ##
==========================================
+ Coverage   90.61%   90.93%   +0.31%     
==========================================
  Files          14       14              
  Lines        1236     1235       -1     
==========================================
+ Hits         1120     1123       +3     
+ Misses        116      112       -4     
Files Changed Coverage Δ
nigsp/operations/laplacian.py 95.78% <100.00%> (+4.12%) :arrow_up:
smoia commented 1 year ago

:rocket: PR was released in 0.18.1 :rocket:

mscheltienne commented 1 year ago

Have a look at assert_allclose from the numpy.testing module: https://numpy.org/doc/stable/reference/generated/numpy.testing.assert_allclose.html

Compared to np.allclose it has a saner atol=0 and more informative message on failure. I got that tip from a trusted Scipy maintainer ;)

smoia commented 1 year ago

Nice! Do you want to open a quick pr to change it?