Random vertex sampling utility function for C++ tests #3347
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
seunghwak
changed the title
…g_test_random_vertex_set
…g_test_random_vertex_set
…g_test_random_vertex_set
…g_test_random_vertex_set
…g_test_random_vertex_set
…elect random vertex subset
seunghwak
changed the title
seunghwak
added
improvement
| size_t sample_size, | ||
| bool with_replacement, | ||
| bool sort_samples) | ||
| { |
There was a problem hiding this comment.
Should we perhaps use
, or combine these by adding thesort_samples parameter to that function and perhaps using this implementation? Seems like the functionality should be the same.
There was a problem hiding this comment.
Oh, yes, you're right. I forgot about the function.
…g_test_random_vertex_set
ChuckHastings
approved these changes
Mar 24, 2023
…me a random vertex generation function returns a fixed known vertices
…g_test_random_vertex_set
naimnv
reviewed
Mar 27, 2023
Comment on lines
+133
to
+141
| if (min_buffer_size <= select_count / comm_size) { | ||
| auto new_sizes = std::vector<size_t>(comm_size, min_buffer_size); | ||
| auto num_deficits = select_count - min_buffer_size * comm_size; | ||
| for (int i = 0; i < comm_size; ++i) { | ||
| auto delta = std::min(num_deficits, buffer_sizes[i] - min_buffer_size); | ||
| new_sizes[i] += delta; | ||
| num_deficits -= delta; | ||
| } | ||
| this_gpu_select_count = new_sizes[comm_rank]; |
There was a problem hiding this comment.
I wonder when would this be necessary?
There was a problem hiding this comment.
Say we need to select 798 vertices out of 800 vertices.
And after shuffling 800 vertices, GPU0 has 380 vertices and GPU 1 has 420 vertices.
We can't select 798/2=399 vertices in GPU0, so we need to select 380 in GPU0 and 418 in GPU1.
naimnv
approved these changes
Mar 27, 2023
|
/merge |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Code clean-up and use a common vertex sampling function throughout C++ tests.
Additional bug fixes
aa25afb
290751f