-
-
Notifications
You must be signed in to change notification settings - Fork 450
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 parallel::partition. #2778
Conversation
… to ensure the way swap_ranges works in parallel::partition.
1. Remove non-meaning unnamed namespace. 2. Use HPX_CONCEPT_REQUIRES_ instead of explicit std::enable_if. 3. Put codes for parallel partition into partition_helper.
…es of parallel::partition.
…l_number_of_chucks(...).
When working on various scan based algorithms (and also parallel::sort), I came to the conclusion that it was a difficult problem to decide what the 'best' chunk/block size to use would be without first running the algorithm to benchmark it with a few test sizes. My own view is that we should have a cmake integrated set of tests for certain of the parallel algorithms (and others if desired), that runs those algorithms and generates a config.h file in the build directory that is then picked up during the main build of hpx. the idea would be that a user downloads hpx and builds it and gets 'default' values for the block sizes etc., but if they run a helper application that is also compiled - with a name like - hpx_platform_optimization(.exe) then the algorithms will be run with some adaptive checking that detects the optimal (or at least a reasonable) value for certain params like block size, and dumps these out to the config file. the user can then recompile hpx and have some faith that it has been tweaksed a bit to work well on that machine. |
@biddisco I think that your idea is good. But, if we decide to do those works, those works should be progressed in individual PR. |
@taeguk correct: Any block/chunk size optimization would be orthogonal to the main algorithm development and contained in its own branch (preferably once the algorithms are all implemented and reliably tested) |
@hkaiser I'm all finished except the thing which enables the user to control the block size. |
@taeguk What should we do with this? Any ideas how to re-introduce controlling the block-size? |
@hkaiser Unfortunatelly, I waited @mcopik 's answer to my comment above. I suggest merging this PR for now, and resolving that issue later. |
@taeguk The value @hkaiser If we intend to use chunk size as a generic interface for controlling all parallel algorithms, then the documentation needs an update. Docs should clearly specify which chunk types are expected and supported for a given algorithm. Furthermore, parameters docs have to be updated because right now the We may even give some thought to renaming chunker types. In this case, it makes sense to use a dynamic chunk size, since it's the only one which allows the user to pass a block size directly to the algorithm. However, the name doesn't really describe the purpose. The block size here is not dynamic, it's completely static. |
Ok, fair enough. Could you please create a ticket reminding us of this? |
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.
LGTM, thanks!
@taeguk Merging this to master has caused several problems:
and possibly more (search for |
@taeguk ping? Will you be able to look into the test failures on master? Please fix those problems as soon as possible to allow for all tests to pass. |
@hkaiser Very sorry. I'm now fixing. I'll send PR within 2 hours. |
@taeguk there is another test-error caused by the partition_range test which only shows up when using clang. Please see here for the details: http://rostam.cct.lsu.edu/builders/hpx_clang_3_9_1_boost_1_61_centos_x86_64_debug/builds/140/steps/build_unit_tests/logs/stdio Would you be able to fix this, please? |
@hkaiser Oh, very very sorry to my mistake. That is an error which is caused by omission of 'const' keyword. It is same to past compile error. |
@taeguk no worries, that's what we have the tests for! Your work is absolutely appreciated! |
This is related to #1141
Check Box
- [ ] Enable that the user can control block size.(see #2803)Issues
Notes for future