Skip to content

Conversation

@pvaneck
Copy link
Contributor

@pvaneck pvaneck commented Jun 8, 2020

This adds the OneHot operation to the WebAssembly backend.

Closes: #3114


This change is Reviewable

@pvaneck pvaneck changed the title {WASM} Add OneHot op [WASM] Add OneHot op Jun 8, 2020
@tafsiri tafsiri requested a review from annxingyuan June 17, 2020 15:08
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.

Thanks @pvaneck ! Just a few minor things.

}

registerKernel({
kernelName: 'OneHot',
Copy link
Contributor

Choose a reason for hiding this comment

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

Import OneHot from core as well and use here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

const res = ENGINE.runKernelFunc(
forward, inputs as unknown as NamedTensorMap, null /* grad */, OneHot,
attrs as unknown as NamedAttrMap);
return reshape(res, outShape);
Copy link
Contributor

Choose a reason for hiding this comment

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

We would like things like reshape to happen within the kernel as well. Could you add logic to reshape in the WASM OneHot typescript implementation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I modified the one hot op in core a bit so that flatten is moved into the forward function. To get the correct output shape in the WASM op, I need to pass in the original indices tensor (i.e. not flattened). I hope this is fine.

@pvaneck
Copy link
Contributor Author

pvaneck commented Jun 18, 2020

@annxingyuan Thanks for the review! Please take another look when you have a chance.

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.

Awesome. Thanks @pvaneck !

@annxingyuan annxingyuan merged commit c14d1de into tensorflow:master Jun 18, 2020
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.

[WASM] Error: 'oneHot' not yet implemented or not found in the registry

3 participants