barahona-research-group / PyGenStability

PyGenStability: Multiscale community detection with generalized Markov Stability
https://barahona-research-group.github.io/PyGenStability/
GNU General Public License v3.0
32 stars 11 forks source link

Scale selection #52

Closed d-schindler closed 2 years ago

d-schindler commented 2 years ago

I realised that the current implementation of the scale selection criterion does not recover all local minima of the criterion and that the criterion looks different to my original implementation. Therefore, I replaced the code with my original implementation. In particular, the moving mean is computed with pandas using a triangular window and this seems to be different to the numpy method used previously. However, we should get rid off pandas later.

codecov-commenter commented 2 years ago

Codecov Report

Merging #52 (2a61121) into master (f00fd8e) will increase coverage by 8.14%. The diff coverage is 60.30%.

@@            Coverage Diff             @@
##           master      #52      +/-   ##
==========================================
+ Coverage   41.85%   50.00%   +8.14%     
==========================================
  Files           8        8              
  Lines         571      586      +15     
==========================================
+ Hits          239      293      +54     
+ Misses        332      293      -39     
Flag Coverage Δ
pytest 50.00% <60.30%> (+8.14%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/pygenstability/plotting.py 0.00% <0.00%> (ø)
src/pygenstability/optimal_scales.py 50.72% <50.72%> (ø)
src/pygenstability/constructors.py 100.00% <100.00%> (+9.40%) :arrow_up:
src/pygenstability/pygenstability.py 100.00% <100.00%> (+9.30%) :arrow_up:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

arnaudon commented 2 years ago

@d-schindler , I'm not sure this works, on the multiscale example it does not as compared to version on master branch. I'll let you fix it before we can merge

d-schindler commented 2 years ago

I made a small modification and it should work on our multiscale example. But it would be good to test it on other graphs and compare to the version of @peach-lucien

d-schindler commented 2 years ago

as scale selection has become a more central aspect of the whole package, I moved the file from contrib to pygenstability

arnaudon commented 2 years ago

@d-schindler , if this goes into the main package, could you add tests to it? thank you!

arnaudon commented 2 years ago

ah sorry, as it's called in the main function, it is tested, except the plotting, which we'll add later once the package it more stable, we can merge then!