Skip to content
This repository was 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 requested review from nsthorat and dsmilkov April 25, 2018 19:20
@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
Development

Successfully merging this pull request may close these issues.

2 participants