On Sun, Dec 28, 2014 at 1:40 PM, Asbjørn wrote:

Hi,

not a proper review, just some comments from a library user based on studying documentation and writing a couple of simple programs using Boost.Compute on Intel's and NVIDIA's OpenCL implementations (i7 CPU and GTX970 GPU respectively). I used MSVC2013 for the tests.

Some of these comments I've shared previously on this list, but I'm repeating them here for completeness.

I have some background in heterogeneous development and OpenCL in particular, but I'm no expert. I found Boost.Compute easy to pick up and providing a nice C++ layer, with a nice mix of low and high level.

Comments in no particular order:

1) As a library user and one who's not a C++ guru by any means, the error messages thrown up when passing incorrect parameters can be quite confusing.

For example, first time I tried to use Boost.Compute I managed to not pass the ouput iterator parameter to transform(), so that the lambda expression ended up in that position instead. This resulted in several very confusing errors inside result_of.hpp, with no errors anywhere near my or Boost.Compute source code.

It would be nice if the compilation could fail in more informative manners.

Fully agree. There are many places where more static_assert's could be added to help improve compile error messages. I'll work on this.

2) I did miss async versions of the algorithms, so it's possible to chain together multiple calls. Even though all the data sits on the compute device, the overhead of waiting for each operation to finish before queuing the next can make the compute gains completely irrelevant.

Can you let me know what chain of functions you're calling? Many algorithms should already execute asynchronously and provide the behavior you expect.

3) I think relevant calls should have a non-throwing form returning an error code, ala Boost.ASIO.

This could be implemented, but would be a large amount of work (essentially doubling the size of the public API). Can you let me know more about your use-case and why the current exception-based API is not suitable?

4) Threading has been mentioned in this list. At the very least the documentation should be updated to clearly state the thread safety of each call/class, ala Boost.ASIO. Beyond that I would like to be able to share a context between several threads, each which could contain their own queue and device.

I'll add more documentation for this. You should be able to share a context between multiple threads already, let me know if you run into any issues with this.

5) If thread safety or similar important features requires a compiled part, I wouldn't mind Boost.Compute being non-header-only. Though if possible making it optional like other Boost libraries do would be great.

I have been considering this more, but it would be a large change (and would be a non-backwards-compatible for existing users).

6) The tests, and preferably the performance tests as well, should have some way of testing all devices across all platforms. Currently they only test the default device on the default platform it seems. This is insufficient IMHO.

I have wanted to improve the testing infrastructure to run all tests for all devices automatically, I just don't know the best way to use Boost.Test to accomplish this. For now I manually run the tests for each device (usually by setting BOOST_COMPUTE_DEFAULT_DEVICE and then running ctest).

7) I'd like some way of defining user functions either as lambda expressions, raw OpenCL-C code via make_function_from_source or both, which can be used in lambda expressions for say transform(). Something like

function sqr = _1 * _1;

transform(inp.begin(), inp.end(), out.begin(), sqr(2 * _1 + 1)), queue);

This is definitely something I want to support, just haven't had the time to implement it yet. There is an issue open already for it [1].

Overall I think Boost.Compute can be useful right now for some use-cases, and it has good potential to be significantly more useful with a bit more work.

Thanks. I'll definitely continue working to fix these issues (as well as others that came up during the review). -kyle [1] https://github.com/kylelutz/compute/issues/59