[wasm] Add CropAndResize kernel.

[annxingyuan]

To see the logs from the Cloud Build CI, please join either our discussion or announcement mailing list.

---

This change is 

[nsthorat]

Reviewed 3 of 6 files at r1, 3 of 3 files at r2.
Reviewable status: 0 of 1 approvals obtained (waiting on @annxingyuan and @nsthorat)

---

tfjs-backend-wasm/src/setup_test.ts, line 39 at r2 (raw file):

    ]
  },
  {include: 'cropAndResize', excludes: []},

you dont need to define excludes

---

tfjs-backend-wasm/src/cc/kernels/CropAndResize.cc, line 56 at r2 (raw file):

    for (int c = 0; c < num_channels; ++c) {
      int ind = c + left_ind * images_strides[2] + top_ind * images_strides[1] +

use const where you don't change values. here above and below

---

tfjs-backend-wasm/src/kernels/CropAndResize.ts, line 29 at r2 (raw file):

interface CropAndResizeAttrs extends NamedAttrMap {
  method: InterpolationMethod;

should this be keyof InterpolationMethod to avoid the cast below?

---

tfjs-backend-wasm/src/kernels/CropAndResize.ts, line 103 at r2 (raw file):

      imageStrides.length, outputStridesBytes, outputStrides.length,
      imagesShapeBytes, images.shape.length, cropSizeBytes,
      (cropSize as [number, number]).length,

isn't this always 2? should this be number[]?

[annxingyuan]

Reviewable status: 0 of 1 approvals obtained (waiting on @annxingyuan and @dsmilkov)

---

tfjs-backend-wasm/src/cc/kernels/CropAndResize.cc, line 20 at r1 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

remove stdio. Also if you do need math, include it as #include <cmath> which is the c++ style

Done

---

tfjs-backend-wasm/src/cc/kernels/CropAndResize.cc, line 25 at r1 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

use ALL_CAPS for the values inside the enum (or you can name them as constants with a starting k). See https://google.github.io/styleguide/cppguide.html#Enumerator_Names

Done

---

tfjs-backend-wasm/src/cc/kernels/CropAndResize.cc, line 56 at r1 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

now that #2342 is in, use backend::get_tensor_info_out() for the output, so that you can modify it (it returns non-const reference).

Done

---

tfjs-backend-wasm/src/cc/kernels/CropAndResize.cc, line 58 at r1 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

now that #2342 is in, use images_info.f32() instead. here and below

Done

---

tfjs-backend-wasm/src/cc/kernels/CropAndResize.cc, line 145 at r1 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

int(float) will floor the float. just double checking that this is intended

Yes, we only memcpy if there is no interpolation required. There is no interpolation required if y_ind has no fractional component.

---

tfjs-backend-wasm/src/cc/kernels/CropAndResize.cc, line 152 at r1 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

for readability, can you move the bilinear code in this if branch to a different method (e.g. bilinearInterp), and the code in the else branch to another one (e.g. nearestInterp). Place these methods in an unnamed namespace to make them private: https://google.github.io/styleguide/cppguide.html#Unnamed_Namespaces_and_Static_Variables

Done

---

tfjs-backend-wasm/src/cc/kernels/CropAndResize.cc, line 56 at r2 (raw file):

Previously, nsthorat (Nikhil Thorat) wrote…

use const where you don't change values. here above and below

Done

---

tfjs-backend-wasm/src/kernels/CropAndResize.ts, line 23 at r1 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

images, boxes, boxInd

Done

---

tfjs-backend-wasm/src/kernels/CropAndResize.ts, line 27 at r1 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

method, extrapolationValue, cropSize

(i think we should remove NamedAttrMap in a future PR since it provides no type-safety at all)

Done

---

tfjs-backend-wasm/src/kernels/CropAndResize.ts, line 52 at r1 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

compute the output shape in c++ (minimize js/c++ transfer)

Done

---

tfjs-backend-wasm/src/kernels/CropAndResize.ts, line 54 at r1 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

since you are already passing images.shape, don't pass images.strides. Instead compute the strides using util::compute_strides method in c++

Done

---

tfjs-backend-wasm/src/kernels/CropAndResize.ts, line 29 at r2 (raw file):

Previously, nsthorat (Nikhil Thorat) wrote…

should this be keyof InterpolationMethod to avoid the cast below?

Done

Although I still seem to have to cast below?

---

tfjs-backend-wasm/src/kernels/CropAndResize.ts, line 103 at r2 (raw file):

Previously, nsthorat (Nikhil Thorat) wrote…

isn't this always 2? should this be number[]?

Done

---

tfjs-core/src/ops/image_ops.ts, line 306 at r1 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

extract the null as a const der or do null /* der */ for readability

Done

[annxingyuan]

Reviewable status: 0 of 1 approvals obtained (waiting on @dsmilkov)

---

tfjs-backend-wasm/src/setup_test.ts, line 39 at r2 (raw file):

Previously, nsthorat (Nikhil Thorat) wrote…

you dont need to define excludes

Done

[dsmilkov]

Thank you! Great work

Reviewed 5 of 5 files at r3.
Reviewable status: complete! 1 of 1 approvals obtained (waiting on @annxingyuan, @dsmilkov, and @nsthorat)

---

tfjs-backend-wasm/scripts/cpplint.js, line 32 at r3 (raw file):

const filenameArgument = result.join(' ');
exec(`python tools/cpplint.py --root ${cwd} ${filenameArgument}`);

I changed this to python2 on purpose since cpplint.py only works correctly with python 2.x. If your python maps to python 3.x, the lint script doesn't detect any errors.

Can you check the python version and error if it is 3.x letting the user know they need to install python2? You can do this by executing python --version and parsing the output

[annxingyuan]

Reviewable status: complete! 1 of 1 approvals obtained (waiting on @nsthorat)

---

tfjs-backend-wasm/scripts/cpplint.js, line 32 at r3 (raw file):

Previously, dsmilkov (Daniel Smilkov) wrote…

I changed this to python2 on purpose since cpplint.py only works correctly with python 2.x. If your python maps to python 3.x, the lint script doesn't detect any errors.

Can you check the python version and error if it is 3.x letting the user know they need to install python2? You can do this by executing python --version and parsing the output

Done