Migrate conv2D and related ops to logical sampling #52

dsmilkov · 2017-08-17T01:40:34Z

This change is

…r compiler

dsmilkov · 2017-08-17T18:04:56Z

Review status: 0 of 39 files reviewed at latest revision, 2 unresolved discussions.

src/math/webgl/conv_gpu.ts, line 54 at r2 (raw file):

          float xR = xRCorner + wR;

          if (xR < 0.0 || xR > ${xRowsLimit}) {

decided to not use zeropadding since the shader is 15% slower than an if statement, as measured on the benchmark, and if statements are easier to read.

src/math/webgl/conv_gpu_test.ts, line 284 at r1 (raw file):

  });

  it('2x2x1 in, 1d out, 2x2 filter, 1 stride, zeropad = 1', () => {

this test is a duplicate of an earlier test, 3 tests before this.

Comments from Reviewable

nsthorat · 2017-08-17T23:51:24Z

Reviewed 10 of 20 files at r1, 28 of 29 files at r2.
Review status: 38 of 39 files reviewed at latest revision, 11 unresolved discussions.

demos/benchmarks/conv_gpu_benchmark.ts, line 40 at r2 (raw file):

  const program = new Conv2DProgram(
      inputShape, fieldSize, outputDepth, stride, zeroPad, true);

pull true out to a const

demos/benchmarks/conv_transpose_gpu_benchmark.ts, line 41 at r2 (raw file):

  const program = new Conv2DTransposeProgram(
      xShape, fieldSize, origInputDepth, origStride, origPad, false);

pull 'false' out to a const

demos/benchmarks/max_pool_gpu_benchmark.ts, line 28 at r2 (raw file):

export const MAX_POOL_BENCHMARK_TEST: BenchmarkTest = (size: number) => {
  return testMaxPool(size, false);

can you make false / true below a "const positions = ..."

src/math/math_gpu.ts, line 534 at r2 (raw file):

      origPad: number): Array3D {
    const maxPoolPositionsProgram =
        new Pool2DProgram(x.shape, fSize, origStride, origPad, 'max', true);

can you pull 'true' out to a variable

src/math/webgl/conv_backprop_gpu.ts, line 32 at r2 (raw file):

    const yNumRows = yShape[0];
    const yNumCols = yShape[1];
    const xRowsLimit = xShape[0] - 0.5;

if possible, use >= instead of subtracting 0.5

src/math/webgl/conv_gpu.ts, line 33 at r2 (raw file):

    this.params = [fieldSize, stride, pad, hasBias];
    const biasSnippet = hasBias ? 'dotProd += getBias(d2);' : '';
    const xRowsLimit = xShape[0] - 0.5;

qq why -0.5? can we just use a >= like in pool?

src/math/webgl/max_pool_backprop_gpu_test.ts, line 37 at r2 (raw file):

    const positionsProgram =
        new Pool2DProgram(x.shape, fSize, origStride, origPad, 'max', true);

pull true out to a variable

src/math/webgl/max_pool_positions_gpu_test.ts, line 34 at r2 (raw file):

    initializeGPU(gpgpu, textureManager);
    const program =
        new Pool2DProgram(xShape, fieldSize, stride, pad, 'max', true);

pull true out to a variable

src/math/webgl/shader_compiler.ts, line 19 at r2 (raw file):

export type ShapeInfo = {
  logicalShape: number[]; texShape: [number, number];

didn't we want to keep this as a comma so it doesn't force us to put these on the same line? if not, let's just be consistent with the type below

Comments from Reviewable

dsmilkov · 2017-08-18T01:33:36Z

Review status: 29 of 39 files reviewed at latest revision, 9 unresolved discussions, some commit checks pending.

demos/benchmarks/conv_gpu_benchmark.ts, line 40 at r2 (raw file):