-
Notifications
You must be signed in to change notification settings - Fork 446
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
Implement degree assortativity coefficient app #492
Conversation
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.
nice start.
analytical_engine/apps/assortativity/degree/degree_assortativity_coefficient.h
Outdated
Show resolved
Hide resolved
analytical_engine/apps/assortativity/degree/degree_assortativity_coefficient.h
Outdated
Show resolved
Hide resolved
analytical_engine/apps/assortativity/degree/degree_assortativity_coefficient.h
Outdated
Show resolved
Hide resolved
analytical_engine/apps/assortativity/degree/degree_assortativity_coefficient.h
Outdated
Show resolved
Hide resolved
analytical_engine/apps/assortativity/degree/degree_assortativity_coefficient.h
Outdated
Show resolved
Hide resolved
hi, @Ningsir, please assign the cla agreement and pass the ci, especially the code format check. |
using oid_t = typename fragment_t::oid_t; | ||
using edata_t = typename fragment_t::edata_t; | ||
|
||
typedef double degree_t; |
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.
using degree_t = double;
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
analytical_engine/apps/assortativity/degree/degree_assortativity_coefficient.h
Show resolved
Hide resolved
void PEval(const fragment_t& frag, context_t& ctx, | ||
message_manager_t& messages) { | ||
ctx.merge_stage = false; | ||
// vid |
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.
remove unnecessary comments
ctx.merge_stage = false; | ||
// vid | ||
auto inner_vertices = frag.InnerVertices(); | ||
// v of type: Vertex |
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.
ditto
analytical_engine/apps/assortativity/degree/degree_assortativity_coefficient_context.h
Show resolved
Hide resolved
analytical_engine/apps/assortativity/degree/degree_assortativity_coefficient_context.h
Outdated
Show resolved
Hide resolved
analytical_engine/apps/assortativity/degree/degree_assortativity_coefficient_context.h
Outdated
Show resolved
Hide resolved
#define ANALYTICAL_ENGINE_APPS_ASSORTATIVITY_UTILS_H_ | ||
|
||
namespace gs { | ||
enum class DegreeType { IN, OUT, INANDOUT }; |
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.
just put enum to context.h, no need to add a new file
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.
DegreeType and some codes about matrix computation can be reused in numeric assortativity app. I will rafactor and put these codes in utils.h
analytical_engine/apps/assortativity/degree/degree_assortativity_coefficient.h
Outdated
Show resolved
Hide resolved
analytical_engine/apps/assortativity/degree/degree_assortativity_coefficient.h
Outdated
Show resolved
Hide resolved
analytical_engine/apps/assortativity/degree/degree_assortativity_coefficient.h
Outdated
Show resolved
Hide resolved
} | ||
|
||
template <typename T> | ||
degree_t ComputeWeightedDegree(T adjList) { |
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.
add some comment about the function. If the function is simple, we should use inline
analytical_engine/apps/assortativity/degree/degree_assortativity_coefficient.h
Outdated
Show resolved
Hide resolved
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.
hi, I add some reviews on the latest pr.
Can you add a test case with p2p-31? like
GraphScope/python/graphscope/nx/tests/test_ctx_builtin.py
Lines 219 to 221 in 35b2fdf
def test_katz_centrality(self): | |
ans = dict(nx.builtin.katz_centrality(self.p2p, weight="default").values) | |
self.assert_result_almost_equal(ans, self.p2p_kz_ans) |
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.
Good job! Thanks for the contribution.
What do these changes do?
Implement degree assortativity coefficient app with cpp.
Related issue number
#453
Experiment
Experiment setting: