-
Notifications
You must be signed in to change notification settings - Fork 47
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix test laplacian lambda max #242
base: master
Are you sure you want to change the base?
Fix test laplacian lambda max #242
Conversation
Codecov Report
@@ 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
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
This reverts commit 9d43be2.
data = [g for i in 1:5] | ||
gall = Flux.batch(data) | ||
@test laplacian_lambda_max(gall) ≈ [Float32(1.809017) for i in 1:5] | ||
@test length(laplacian_lambda_max(gall)) == 5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@test length(laplacian_lambda_max(gall)) == 5 |
the length is already implicitly tested in the line above
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 |
Yes and also in the function KrylovKit.eigsolve(Symmetric(A), x0, 1, :LR)[1][1] hence A is forced to be symmetric. |
So all this PR does is remove a test... |
With this PR I added the seed for
rand_graph
.