martin-danelljan / ECO

Matlab implementation of the ECO tracker.
GNU General Public License v3.0
614 stars 252 forks source link

Add Octave support #5

Closed cybertk closed 7 years ago

cybertk commented 7 years ago

This PR brings GNU/Octave support

Issues when running in Octave:

Changes:

martin-danelljan commented 7 years ago

I had to temporarily revert the pr since I found that the 'cast(...)' does not work properly with gpuArray. We have to think of a solution for this.

cybertk commented 7 years ago

I only tested the HC version of ECO, as Octave has no support of GPU. What about if we enable only ECO_HC/params.use_gpu = false for Octave, and report a warning for else.

martin-danelljan commented 7 years ago

The problem is not that. Since the GPU version run the same code, the allocation and casting operations need to support gpuArray as well. An ugly fix is of course to add if is_octave ... else ... end around each allocation or casting operation. Another possibility is to add a separate branch for octave support to keep the code cleaner.

cybertk commented 7 years ago

If we provide a customization version of cast() to override system's? In that, we can do if is_octave ... else ... end.

Is it acceptable?

But I thinks we can try to fix the real root-cause. I'm not sure I understand the issue,cast(A, class(B)) does not work if A/B is gpuArray?

cybertk commented 7 years ago

Actually, I'm trying to encapsulate ECO with a thin C/C++ wrapper with liboctave. Then we can integrate it into production code. So the Octave support is the first step of that.

martin-danelljan commented 7 years ago

Sound like a nice idea to integrate it with liboctave.

The problem is essentially that class(gpuArray(0)) only returns 'gpuArray', but not the precision of it, i.e. single or double. This causes cast to simply fail and zeros allocates a gpuArray of type double.

There is also another problem: complex(zeros(...)) is much slower than the zeros(..., 'like', data_type_complex).

martin-danelljan commented 7 years ago

I thought about some different options. I think the best option is to write custom versions of zeros and cast, as you say. The downside is that there is a small overhead for such function calls in Matlab.

I have written one for zeros below. It should be called as: zeros_like(..., 'like', params.data_type, params.is_octave) So, only the is_octave needs to be added as a last argument. The cast_like function could be implemented in a similar way.

function x = zeros_like(varargin)

if varargin{end}
    % Octave
    if isreal(varargin{end-1})
        x = zeros(varargin{1:end-3}, class(varargin{end-1}));
    else
        x = complex(zeros(varargin{1:end-3}, class(varargin{end-1})));
    end
else
    % Matlab
    x = zeros(varargin{1:end-1});
end
end
martin-danelljan commented 7 years ago

Or maybe is_octave can be added as a persistent variable in the zeros_like function. Then it does not even have to be supplied as a parameter.

cybertk commented 7 years ago

I think is_octave should be persistent respected to performance concern