-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[webgl] Fixed split issue with negative axis. #4478
[webgl] Fixed split issue with negative axis. #4478
Conversation
@BruceDai thanks for this, the change looks good but it would be great to have a test with a negative axes param to catch regressions across backends. Would you be able to add one to split_test.ts? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch, thank you Bruce! LGTM. +1 to Yannick's suggestion.
Reviewable status: 0 of 1 approvals obtained
The test failed because of this line, can you also update this line to use $axis? https://github.com/tensorflow/tfjs/blob/master/tfjs-backend-wasm/src/kernels/SplitV.ts#L33 |
@lina128 Done. PTAL, thanks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! LGTM
Reviewable status: complete! 1 of 1 approvals obtained
BUG
To see the logs from the Cloud Build CI, please join either our discussion or announcement mailing list.
This change is