Skip to content

Conversation

@nsthorat
Copy link
Contributor

@nsthorat nsthorat commented Dec 5, 2019

We do this by first casting the input to float32. This happens in V8 so should be very fast.


This change is Reviewable

Nikhil Thorat added 2 commits December 5, 2019 10:57
Nikhil Thorat added 2 commits December 5, 2019 11:12
Copy link
Contributor

@dsmilkov dsmilkov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 8 of 8 files at r1, 1 of 1 files at r2.
Reviewable status: :shipit: complete! 1 of 1 approvals obtained (waiting on @annxingyuan and @dsmilkov)

Copy link
Contributor

@annxingyuan annxingyuan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: :shipit: complete! 2 of 1 approvals obtained (waiting on @annxingyuan, @dsmilkov, and @nsthorat)


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

  let imagesData = backend.dataIdMap.get(images.dataId);
  let castedDataId;

Should we call this castedData instead?


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

        [1, 2, 0, 3, 4, 0, 3, 4, 0, 5, 6, 6, 7, 8, 8, 0, 0, 0]);
  });
  it('int32 image returns float output', async () => {

Just curious does this test fail correctness on master?

Copy link
Contributor Author

@nsthorat nsthorat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: :shipit: complete! 2 of 1 approvals obtained (waiting on @annxingyuan)


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

Previously, annxingyuan (Ann Yuan) wrote…

Should we call this castedData instead?

Done. Good suggestion :)


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

Previously, annxingyuan (Ann Yuan) wrote…

Just curious does this test fail correctness on master?

Yeah it does

@nsthorat nsthorat merged commit 106fedf into master Dec 5, 2019
@nsthorat nsthorat deleted the bresize branch December 5, 2019 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants