-
Notifications
You must be signed in to change notification settings - Fork 217
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
Add triangle_counting tests #1586
Conversation
std::int64_t edge_count; | ||
std::int64_t global_triangle_count; | ||
}; | ||
class complete_graph_5_type : public graph_base_data { |
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.
Does it make sense to share these graphs between several algorithms?
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.
Agree
std::array<std::int64_t, 6> local_triangles = { 5, 2, 2, 2, 2, 2 }; | ||
}; | ||
|
||
class isolated_vertices_graph_10_type : public graph_base_data { |
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.
It is not isolated according to the rows and cols fields.
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.
What graph do you call "isolated vertices"?
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.
Graph that contains isolated vertices 3, 6 and 8
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.
Do you mean that it makes sense to rename graph to "graph_with_isolated_vertices_10_type"?
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.
Yes.
std::array<std::int64_t, 10> local_triangles = { 3, 2, 1, 0, 2, 4, 0, 1, 0, 2 }; | ||
}; | ||
|
||
class isolated_vertex_graph_11_type : public graph_base_data { |
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.
same
|
||
const std::int64_t vertex_count = graph_data.get_correct_vertex_count(); | ||
const std::int64_t edge_count = graph_data.get_correct_edge_count(); | ||
const std::int64_t cols_count = edge_count * 2; |
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.
Can you store it in the graph class? Without magic x2 which is related to the concrete representation of an undirected graph.
That is cols_count = graph_data.get_cols_count()
vertex_allocator, | ||
rows_count); | ||
|
||
std::int32_t *degrees = new (degrees_) std::int32_t[vertex_count]; |
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.
Why do you need placement new?
vertex_allocator, | ||
cols_count); | ||
std::int64_t *rows_ = | ||
std::allocator_traits<std::allocator<char>>::rebind_traits<std::int64_t>::allocate( |
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.
Use detail::allocate alias
float, | ||
dal::preview::triangle_counting::method::ordered_count, | ||
dal::preview::triangle_counting::task::global, | ||
std::allocator<char>>(alloc); |
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.
set descriptor option relabel::no to be sure.
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.
It is done on line 338. Is it incorrect?
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.
line 338 seems to be outdated. API of TC: auto desc = descriptor<>().set_relabel(triangle_counting::relabel::no);
Ok, It is fixed below.
/intelci: run |
|
||
TEST_M(triangle_counting_test, "global task for relabeled graph with average_degree >= 4") { | ||
this->check_global_task_relabeled<complete_graph_9_type>(); | ||
//this->check_global_task_relabeled<isolated_vertex_graph_11_type>(); |
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 the obsolete comments
std::int64_t edge_count; | ||
std::int64_t global_triangle_count; | ||
}; | ||
class complete_graph_5_type : public graph_base_data { |
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.
Agree
std::array<std::int64_t, 6> local_triangles = { 5, 2, 2, 2, 2, 2 }; | ||
}; | ||
|
||
class isolated_vertices_graph_10_type : public graph_base_data { |
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.
Yes.
float, | ||
dal::preview::triangle_counting::method::ordered_count, | ||
dal::preview::triangle_counting::task::global, | ||
std::allocator<char>>(alloc); |
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.
line 338 seems to be outdated. API of TC: auto desc = descriptor<>().set_relabel(triangle_counting::relabel::no);
Ok, It is fixed below.
std::allocator<char>>(alloc); | ||
|
||
tc_desc.set_relabel(dal::preview::triangle_counting::relabel::no); | ||
const auto relabel = tc_desc.get_relabel(); |
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.
Why do you need this assignment and check after?
No description provided.