Skip to content
This repository has been archived by the owner on Aug 15, 2019. It is now read-only.

Add implementation for resizeBilinear gradient. #996

Merged
merged 11 commits into from May 8, 2018

Conversation

tafsiri
Copy link
Contributor

@tafsiri tafsiri commented Apr 25, 2018

Also fixes a bug in resizeBilinear.


This change is Reviewable

@tafsiri tafsiri changed the title Add CPU implementation for resizeBilinear gradient. Add implementation for resizeBilinear gradient. May 2, 2018
@tafsiri
Copy link
Contributor Author

tafsiri commented May 2, 2018

This now also has a GPU implementation!

@dsmilkov
Copy link
Contributor

dsmilkov commented May 3, 2018

Reviewed 7 of 7 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.


src/kernels/backend_cpu.ts, line 1499 at r1 (raw file):

    for (let b = 0; b < batch; b++) {
      for (let r = 0; r < yHeight; r++) {
        const inY = r * heightScale;

keep row column notation. Since y and x are reserved for input tensor and output tensor.


src/kernels/backend_cpu.ts, line 1514 at r1 (raw file):

          for (let d = 0; d < depth; d++) {
            let topLeft = output.get(b, topYIndex, leftXIndex, d);
            topLeft += dy.get(b, r, c, d) * inverseYLerp * inverseXLerp;

Try to use direct flat indexing into the values (avoid calling .get) to speed it up. See https://reviewable.io/reviews/tensorflow/tfjs-core/995 for how conv2d got 100x faster.


src/kernels/webgl/resize_bilinear_backprop_gpu.ts, line 78 at r1 (raw file):

        // Loop over dy
        for (int dry = 0; dry < int(${winHeight}); dry++) {

extract in a variable int(${winHeight} so you don't cast every tick of the for loop. Here and the for loop below.


src/kernels/webgl/resize_bilinear_backprop_gpu.ts, line 79 at r1 (raw file):

        // Loop over dy
        for (int dry = 0; dry < int(${winHeight}); dry++) {
          int ry = dry + startRY;

in conv2d gpu we do dyR/dyC and row and column in dy.


src/ops/image_ops.ts, line 61 at r1 (raw file):

    const [newHeight, newWidth] = size;
    const forward: ForwardFunc<Tensor4D> = (backend, save) => save(

Don't save the result. Use shape from dy


src/ops/image_ops.ts, line 67 at r1 (raw file):

      const y = saved[0] as Tensor4D;

      const result = ENV.engine.runKernel(

make result a function that gets called by someone else. That is const result = () => ENV.engine.runKernel(...


src/ops/image_ops.ts, line 69 at r1 (raw file):

      const result = ENV.engine.runKernel(
          backend =>
              backend.resizeBilinearGrad(dy, batchImages, y, alignCorners),

no need to pass y, which means no need you have y = saved[0]


Comments from Reviewable

@dsmilkov
Copy link
Contributor

dsmilkov commented May 3, 2018

Review status: all files reviewed at latest revision, 7 unresolved discussions, some commit checks failed.


src/ops/resize_bilinear_test.ts, line 179 at r1 (raw file):

      [[9.0], [10.0], [11.0], [12.0]], [[13.0], [14.0], [15.0], [16.0]]
    ]);
    const g = tf.grad(resize.bind(null, [4, 4], false));
const size = [4, 4];
const alignCorners = false;
const g = tf.grad(image => tf.image.resizeBilinear(image, size, alignCorners));
g(image, dy);

Comments from Reviewable

@tafsiri
Copy link
Contributor Author

tafsiri commented May 3, 2018

Ready for re-review


Review status: 1 of 7 files reviewed at latest revision, 8 unresolved discussions, some commit checks failed.


src/kernels/backend_cpu.ts, line 1499 at r1 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

keep row column notation. Since y and x are reserved for input tensor and output tensor.

Done.


src/kernels/backend_cpu.ts, line 1514 at r1 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

Try to use direct flat indexing into the values (avoid calling .get) to speed it up. See https://reviewable.io/reviews/tensorflow/tfjs-core/995 for how conv2d got 100x faster.

Not yet done. If this PR looks good i'd like to try do that in a separate PR (just so that this one doesn't delay in the mean time).


src/kernels/webgl/resize_bilinear_backprop_gpu.ts, line 78 at r1 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

extract in a variable int(${winHeight} so you don't cast every tick of the for loop. Here and the for loop below.

Done.


src/kernels/webgl/resize_bilinear_backprop_gpu.ts, line 79 at r1 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

in conv2d gpu we do dyR/dyC and row and column in dy.

Done.


src/ops/image_ops.ts, line 61 at r1 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

Don't save the result. Use shape from dy

Done.


src/ops/image_ops.ts, line 67 at r1 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

make result a function that gets called by someone else. That is const result = () => ENV.engine.runKernel(...

Done.


src/ops/image_ops.ts, line 69 at r1 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

no need to pass y, which means no need you have y = saved[0]

Done.


src/ops/resize_bilinear_test.ts, line 179 at r1 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…
const size = [4, 4];
const alignCorners = false;
const g = tf.grad(image => tf.image.resizeBilinear(image, size, alignCorners));
g(image, dy);

Done.


Comments from Reviewable

@dsmilkov
Copy link
Contributor

dsmilkov commented May 5, 2018

:lgtm_strong: Nice work Yannick!


Reviewed 6 of 6 files at r2.
Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.


Comments from Reviewable

@tafsiri tafsiri merged commit 7abc301 into master May 8, 2018
@nsthorat nsthorat deleted the add-resizeBilinear-gradient branch May 15, 2018 17:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants