8000 Feature request: TensorPadOp · Issue #5471 · Theano/Theano · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Feature request: TensorPadOp #5471

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.

Sign up for GitHub

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

Jump to bottom
Closed
khaotik opened this issue Jan 31, 2017 · 3 comments
Closed

Feature request: TensorPadOp #5471

khaotik opened this issue Jan 31, 2017 · 3 comments

Comments

@khaotik
Copy link
Contributor
khaotik commented Jan 31, 2017

numpy 1.7+ has np.pad

Use case

  • special convolution kernel
    Assume conv an image with kernel [[-1., 0., 1.]], instead of using conv2d, one could do:

    x = T.matrix()
fn_pad = partial(T.pad, mode='constant')
fn_pad(x, ((0,0), (1, -1))) - fn_pad(x, ((0,0), (-1, 1)))

    This would be useful for simulating PDEs.
    This can also be done with T.join(zeros(...), ...) but I'm not sure about performance.
    np.pad does not support negative padding, but it does not mean Theano shouldn't support it. More explained below.

  • cyclic convolution
    cuDNN does not have API for this, so this can be done as conv2d(pad(..., mode='wrap'))
    This could be useful for certain PDE boundary conditions or texture synthesis.

  • Replaces the current theano.roll, which uses subtensor + join

    x = T.vector()
T.pad(x, ((-1,1)), mode='wrap') # rotate left by 1

Implementation ideas

  • Fuse with elemwise Op

In most cases, pad behaves as an injection, and sometimes bijection. I think it's good to generate code that can be fused with elemwise Op. The above convolution example should compile into a single kernel like:

__global__ dx_kernel(float *x, size_t stride_x float *y, size_t stride_y, size_t N) {
    int i = blahblahblah;
    float left, right;
    left = i<0 ? 0.f : x[(i-1)*x_stride]; // from TensorPadOp
    right = i>=N ? 0.f : x[(i+1)*x_stride]; // from TensorPadOp
    y[i * y_stride] = left - right; // from Elemwise
}

With this, we can construct cross shaped convolution, bilateral filters, high dimensional conv ... any kind of filter, without worrying about performance.

  • Negative padding support

This makes code simpler.
Without negative padding:

fn_pad = partial(pad, mode='constant')
if shift>0:
    y = fn_pad(x, ((shift,0)))[:-shift]
elif shift<0:
   y = fn_pad(x, ((0, shift)))[shift:]

With negative padding:

y = pad(x, ((shift, -shift)), mode='constant')

Note the shape of x and y are same, giving possibility for inplace optimization.

Negative padding is supported in Mathematica's ArrayPad and I find it very convenient.
@khaotik khaotik mentioned this issue Feb 28, 2017
Open
@nouiz
Copy link
Member
nouiz commented Mar 2, 2017

See related issue: #5500. Do you think it would solve well your problem for the performance part? I think for 1d, with subtensor and concatenate, we should be performant enough. But for 2d, probably not.

Did you do timing with the current ops we have? Maybe it is fast enough even if not super efficient as the bottleneck should still be the convolution itself. So maybe we don't care much about the efficiency of this implementation.

@khaotik
Copy link
Contributor Author
khaotik commented Mar 2, 2017

@nouiz The whole starting point of introducing "elemwise" padding is that I'd like to define some custom filters like in #5618. I'm already using subtensor + concat + img2neib in my current implementation. For small models this is sufficient. I suppose there should be substantial speed up if everything is done in a single kernel, but I really didn't test this as I don't have a working implementation yet.

Merging multiple split / concat gives speed up for the padding part but not for the custom filtering part.

I might work on this if I've got the time. I'll let you know when a PoC is ready and put up profiling info.

@mharradon mharradon mentioned this issue Apr 9, 2017
Open
@khaotik
Copy link
Contributor Author
khaotik commented Jul 1, 2017

Just found out this is mostly dup of #1216, closing.

@khaotik khaotik closed this as completed Jul 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@nouiz @khaotik
0