amd / OpenCL-caffe

This is a Experimental version of OpenCL by AMD Research, we now recommend you to use The official BVLC Caffe OpenCL branch is over at Caffe branch now at https://github.com/BVLC/caffe/tree/opencl
Other
517 stars 152 forks source link

`const Dtype* A, const int offA` should be wrapped as an object #4

Closed hughperkins closed 9 years ago

hughperkins commented 9 years ago

Many calls in caffe like:

something(Dtype *A, ...);

have become in caffe fork:

somehting(Dtype* A, const int offA);

I reckon these should be combined into a single object you pass around:

something(DBuffer *A)

where:

class DBuffer {
public:
    Dtype *array;
    int offset;
};

(I reckon that should probably be a long long, or preferably int64_t, by the way, rather than an int, but I confess I use ints mostly too, since I havent hit buffers greater than 8GB or so. But you will... :-P)

gujunli commented 9 years ago

thank you for describing your idea here. for the current version, we may or may not to integrate your idea. As we just want to keep thing simple...and we don't have CUDA in this release. I will think more about it.

keryell commented 9 years ago

Well, it depends on how you think about "keep[ing] thing simple". Using C++ DBuffer without changing most of the code instead of inserting C low-level spaghetti code with const Dtype* A, const int offA everywhere may be actually simpler. :-) If using Boost.Compute for example, the conversion from DBuffer to the 2 arguments would occur only in the last generic template in a transparent way, if you use a clean unified way to call OpenCL kernels. By the way offset should be std::size_t (or std::intptr_t or std::ptrdiff_t if signed needed) instead of int, long long or int64_t.

hughperkins commented 9 years ago

By the way offset should be std::size_t (or std::intptr_t or std::ptrdiff_t if signed needed

Ah interesting :-)