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

Fix ENV.get('EPSILON') and add a unit test #1239

Merged
merged 2 commits into from
Aug 21, 2018
Merged

Fix ENV.get('EPSILON') and add a unit test #1239

merged 2 commits into from
Aug 21, 2018

Conversation

dsmilkov
Copy link
Contributor

@dsmilkov dsmilkov commented Aug 21, 2018

Epsilon was 1e-4 for 32bit, and 1e-8 for 16bit, which is the opposite of the correct value.

BUG


This change is Reviewable

@dsmilkov dsmilkov requested a review from caisq August 21, 2018 17:16
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 quick fix!

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


src/environment.ts, line 326 at r1 (raw file):

      }
      return EPSILON_FLOAT16;

optional style nit: does it make sense to use if/else here?

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.

Reviewable status: :shipit: complete! 1 of 1 approvals obtained (waiting on @dsmilkov and @caisq)


src/environment.ts, line 326 at r1 (raw file):

Previously, caisq (Shanqing Cai) wrote…
      }
      return EPSILON_FLOAT16;

optional style nit: does it make sense to use if/else here?

Or use the ternary operator.

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.

Reviewed 2 of 2 files at r1, 1 of 1 files at r2.
Reviewable status: :shipit: complete! 1 of 1 approvals obtained (waiting on @caisq)


src/environment.ts, line 326 at r1 (raw file):

Previously, caisq (Shanqing Cai) wrote…

Or use the ternary operator.

Done. Switched to ternary op.

@dsmilkov dsmilkov merged commit 6944179 into master Aug 21, 2018
@dsmilkov dsmilkov deleted the fix-epsilon branch August 21, 2018 20:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants