CarloLucibello / GraphNeuralNetworks.jl

Graph Neural Networks in Julia
https://carlolucibello.github.io/GraphNeuralNetworks.jl/dev/
MIT License
216 stars 46 forks source link

Fix test laplacian lambda max #242

Open aurorarossi opened 1 year ago

aurorarossi commented 1 year ago

With this PR I added the seed for rand_graph.

codecov[bot] commented 1 year ago

Codecov Report

Merging #242 (55fd2b4) into master (1381a78) will increase coverage by 0.13%. The diff coverage is 93.75%.

@@            Coverage Diff             @@
##           master     #242      +/-   ##
==========================================
+ Coverage   88.29%   88.42%   +0.13%     
==========================================
  Files          16       16              
  Lines        1597     1624      +27     
==========================================
+ Hits         1410     1436      +26     
- Misses        187      188       +1     
Impacted Files Coverage Δ
src/GNNGraphs/gnnheterograph.jl 85.91% <93.75%> (+4.09%) :arrow_up:
src/GNNGraphs/transform.jl 96.55% <0.00%> (+0.43%) :arrow_up:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

CarloLucibello commented 1 year ago

The problem this PR is trying to fix is the Laplacian test on nightly CI runs, e.g. here, right?

I don't know why that is happening but in principle computing the top eigenvalues of those rand bidirected graphs should not be problematic since their adjacency matrix is symmetric. Maybe zero-degree nodes cause some problems? Although I don't see why they should

aurorarossi commented 1 year ago

Yes and also in the function _eigmax there is the following line:

 KrylovKit.eigsolve(Symmetric(A), x0, 1, :LR)[1][1]

hence A is forced to be symmetric. When I run the tests on my computer they run smoothly (I have Julia 1.8).

CarloLucibello commented 1 year ago

So all this PR does is remove a test...