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

Change epsilon for 32 bit float to 1e-7 and add couple more unit tests #1244

Merged
merged 4 commits into from
Aug 23, 2018

Conversation

dsmilkov
Copy link
Contributor

@dsmilkov dsmilkov commented Aug 22, 2018

Epsilon for 32 bit floats is changed to 1e-7 so that tf.clip([0, 1], eps, 1-eps) returns [eps, 1-eps] instead of [0, 1].

BUG


This change is Reviewable

@dsmilkov dsmilkov requested a review from caisq August 22, 2018 15:35
Copy link
Collaborator

@caisq caisq left a comment

Choose a reason for hiding this comment

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

Thanks for the fix!

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

@@ -288,4 +289,8 @@ describeWithFlags('epsilon', {}, () => {
const epsilonValue = ENV.backend.floatPrecision() === 32 ? 1e-8 : 1e-4;
Copy link
Contributor

@manrajgrover manrajgrover Aug 22, 2018

Choose a reason for hiding this comment

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

Seems like value in this test should also be changed or build will fail.

Copy link
Contributor Author

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

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


src/environment_test.ts, line 289 at r1 (raw file):

Previously, manrajgrover (Manraj Singh) wrote…

Seems like value in this test should also be changed or build will fail.

Done. Nice catch!

@dsmilkov dsmilkov merged commit d619965 into master Aug 23, 2018
@dsmilkov dsmilkov deleted the fix-eps branch August 23, 2018 03:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants