-
Notifications
You must be signed in to change notification settings - Fork 46
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
C++ to compute conductance #58
Conversation
Pull Request Test Coverage Report for Build 19
💛 - Coveralls |
@@ -403,30 +408,37 @@ def neighbors(self,vertex): | |||
""" | |||
return self.adjacency_matrix[:,vertex].nonzero()[0].tolist() | |||
|
|||
def compute_conductance(self,R): | |||
def compute_conductance(self,R,cpp=True): |
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.
Let's drop this function. The work is the same as set_scores.
@@ -8,6 +8,7 @@ | |||
#include "sparserank.hpp" // include our sorted-list functions | |||
#include "sparsevec.hpp" // include our sparse hashtable functions | |||
#include <tuple> | |||
#include <iostream> |
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.
We shouldn't have iostream here.
@@ -216,6 +218,26 @@ pair<itype, itype> graph<vtype,itype>::get_stats(unordered_map<vtype, vtype>& R_ | |||
return set_stats; | |||
} | |||
|
|||
template<typename vtype, typename itype> | |||
pair<double, double> graph<vtype,itype>::get_stats_weighted(unordered_map<vtype, vtype>& R_map, vtype nR) |
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.
Any reason these are using an unordered_map vs. unordered_set ?
#include <stdlib.h> | ||
#include <unordered_map> | ||
#include <queue> | ||
#include <iostream> |
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.
we should remove iostream
{ | ||
graph<uint32_t,uint32_t> g(ai[n],n,ai,aj,NULL,offset,NULL); | ||
unordered_map<uint32_t, uint32_t> R_map; | ||
for (size_t i = 0; i < nR; i ++) { |
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.
This for loop is duplicated in each of the calls. Can you turn it into a function?
@MengLiuPurdue the right place for the matplotlib.agg is in the test_examples() function in test_notebooks.py So it shouldn't be in the ipynb file, or in the main code... only in the test_notebooks.py file.. |
@@ -29,4 +30,76 @@ def _exec_notebook(notebook_filename): | |||
ep.preprocess(nb, {'metadata': {'path': 'notebooks/'}}) | |||
|
|||
def test_examples(): | |||
_exec_notebook('notebooks/examples.ipynb') | |||
_exec_notebook('notebooks/examples.ipynb') | |||
""" |
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.
We should add a link to where this code came from!
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.
Ok, working on it. I also in the middle of adding more tests to get the coverage higher than 90% hopefully.
int64_t n, int64_t* ai, int64_t* aj, double* a, double* degrees, int64_t offset, | ||
int64_t* R, int64_t nR, double* voltrue, double* cut) | ||
{ | ||
graph<int64_t,int64_t> g(ai[n],n,ai,aj,a,offset,degrees); |
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.
Make sure you convert the tabs to spaces.
@dgleich "So it shouldn't be in the ipynb file, or in the main code... only in the test_notebooks.py file.." I tried, but that didn't seem to work. UPDATE: I found a way to do that by deleting "init.py" inside "tests/" and add those two lines in "conftest.py". Because otherwise, pytest will always import localgraphclustering first. (https://docs.pytest.org/en/latest/pythonpath.html) |
localgraphclustering/cpp/__init__.py
Outdated
@@ -24,7 +24,13 @@ def load_library(): | |||
lib=ctypes.cdll.LoadLibrary(find_path()) | |||
return lib | |||
|
|||
def is_loaded(lib): |
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.
Let's remove this ,it wlll never work on windows.
No description provided.