Skip to content
This repository has been archived by the owner on Aug 15, 2019. It is now read-only.

Revive packed division - add out of bounds check. #1660

Merged
merged 13 commits into from
Apr 3, 2019
Merged

Conversation

annxingyuan
Copy link
Collaborator

@annxingyuan annxingyuan commented Apr 3, 2019

This PR fixes tensorflow/tfjs#1324

This change works with the LSTM text generation example (which is why we turned off packed division to begin with).

Changes

  • Revive packed division
  • Change packed and unpacked division to return NaN if divide by zero
  • Change closeness test to consider NaN and Infinity to be close so when we divide log(0) by 0 and get 0 the test passes

This change is Reviewable

Copy link
Contributor

@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.

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


src/kernels/webgl/binaryop_packed_gpu.ts, line 215 at r1 (raw file):

            bool nextColOutOfBounds =
              (${channels[rank - 1]} + 1) >= ${this.outputShape[rank - 1]};
            

remove the spaces on this line

Copy link
Collaborator Author

@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! 1 of 1 approvals obtained (waiting on @dsmilkov)


src/kernels/webgl/binaryop_packed_gpu.ts, line 215 at r1 (raw file):

Previously, nsthorat (Nikhil Thorat) wrote…

remove the spaces on this line

Done

@annxingyuan annxingyuan merged commit 6e07484 into master Apr 3, 2019
@annxingyuan annxingyuan deleted the div_nan_check branch April 3, 2019 17:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Understand why epsilon checks are necessary in packed division and add test exposing root issue.
2 participants