hughperkins / cltorch

An OpenCL backend for torch.
Other
289 stars 26 forks source link

mean() not implemented / commented out? #23

Closed vkorablin closed 8 years ago

vkorablin commented 8 years ago
th> cpu_tensor:cl():mean()
Using Advanced Micro Devices, Inc. , OpenCL platform: AMD Accelerated Parallel Processing
Using OpenCL device: Capeverde
/home/vkorablin/torch/install/bin/luajit: symbol lookup error: /home/vkorablin/torch/install/lib/lua/5.1/libcltorch.so: undefined symbol: THClTensor_meanall

Looking at the source code https://github.com/hughperkins/cltorch/blob/8e4691765ed6b55108790c9a3189847d7e2439f1/src/lib/THClTensorMath2.cpp#L192 , the implementation is commented out.

Is that a simple oversight is is there a problem with the implementation? Seems fine to me:

void
THClTensor_mean(THClState *state, THClTensor *self, THClTensor *src, long dim)
{
  THAssert(THClTensor_checkGPU(state, 2, self, src));
  THClTensor_sum(state, self, src, dim);
  THClTensor_div(state, self, self, THClTensor_size(state, src, dim));
}

I've uncommented it (and the meanall() implementation above which it apparently requires) and it seems to work just fine.

th> cpu_tensor = torch.rand(2)
                                                                      [0.0000s] 
th> cpu_tensor:cl():mean()
0.62458831071854    
                                                                      [0.0005s] 
hughperkins commented 8 years ago

Thanks. Will take a look.

hughperkins commented 8 years ago

Good point. Added in https://github.com/hughperkins/cltorch/commit/a88f025523c79e954faffb96c470087ad9e072f8

hughperkins commented 8 years ago

(Also added atan2, sign, norm, in same sourcecode file)