Fix modularity when self loops exist#487
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #487 +/- ##
=======================================
Coverage 97.19% 97.19%
=======================================
Files 121 121
Lines 7105 7107 +2
=======================================
+ Hits 6906 6908 +2
Misses 199 199 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
ef9a520 to
9c9fdf7
Compare
Krastanov
left a comment
There was a problem hiding this comment.
Thank you for the submission! You are introducing a very minor slowdown, but on the other hand the code is more legible. I think I am happy to merge it as is. See inline comment for more details.
| Q -= γ * kin[i] * kout[i] | ||
| end | ||
| return Q / m^2 | ||
| Q = Q/m - γ * sum(kin .* kout) / m^2 |
There was a problem hiding this comment.
I imagine whoever wrote the previous inbounds loop wanted to avoid the allocation. The new format creates a temporary array to store kin .* kout before performing the sum. I guess I personally do not care about this type of micro-optimization and prefer your new code, but mentioning this here for documentation purposes.
julia> f(a,b) = sum(a .* b)
f (generic function with 1 method)
julia> g(a,b) = begin s = 0.0; for i in eachindex(a); s += a[i]*b[i]; end; s end
g (generic function with 1 method)
julia> a = rand(100);
julia> b = rand(100);
julia> @time f(a, b);
0.027304 seconds (13.31 k allocations: 711.914 KiB, 99.97% compilation time)
julia> @time f(a, b);
0.000004 seconds (3 allocations: 944 bytes)
julia> @time g(a, b);
0.007733 seconds (5.17 k allocations: 261.992 KiB, 99.91% compilation time)
julia> @time g(a, b);
0.000003 seconds (1 allocation: 16 bytes)
and here are a few more options to consider for simpler syntax
julia> h(a,b) = sum(zip(a,b)) do (x,y); x*y; end
h (generic function with 1 method)
julia> k(a,b) = sum(splat(*), (a,b))
k (generic function with 1 method)
julia> @time h(a, b);
0.018568 seconds (131.63 k allocations: 6.261 MiB, 15.16% gc time, 99.96% compilation time)
julia> @time h(a, b);
0.000002 seconds (1 allocation: 16 bytes)
julia> @time k(a, b);
0.028699 seconds (146.38 k allocations: 7.347 MiB, 99.72% compilation time)
julia> @time k(a, b);
0.000016 seconds (403 allocations: 7.891 KiB)
Fix the modularity function when self loops exist in the graph and some minor formatting to make the modularity computation clearer.