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

Use a more robust algorithm for uniform random variates #1240

Merged
merged 1 commit into from
Aug 27, 2018

Conversation

kgryte
Copy link
Contributor

@kgryte kgryte commented Aug 21, 2018

Description

This PR makes a small modification to the randUniform utility function, which is used when creating a tensor initialized with uniform random variates and during testing.

When a and b are approximately equal, the current algorithm is susceptible to catastrophic cancellation (a truncation of significant digits).

This PR modifies the computation of the random variate by replacing the subtraction of a and b with two multiplications, thus scaling the two input values.

The rearrangement of terms is found elsewhere, such as GSL, etc.

While the PR does add an additional operation (a multiplication), the effect on performance should be negligible on modern hardware and when run in JavaScript (as compared to, e.g., C or Assembly).


For repository owners only:

Please remember to apply all applicable tags to your pull request.
Tags: FEATURE, BREAKING, BUG, PERF, DEV, DOC, SECURITY

For more info see: https://github.com/tensorflow/tfjs/blob/master/DEVELOPMENT.md


This change is Reviewable

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

@kgryte
Copy link
Contributor Author

kgryte commented Aug 21, 2018

I signed the CLA.

@googlebot
Copy link

CLAs look good, thanks!

@nkreeger nkreeger requested a review from dsmilkov August 22, 2018 03:42
Copy link
Contributor

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

Thank you for the amazing contribution!

Reviewed 1 of 1 files at r1.
Reviewable status: 0 of 1 approvals obtained (waiting on @dsmilkov)

@dsmilkov dsmilkov merged commit 9e87791 into tensorflow:master Aug 27, 2018
@kgryte kgryte deleted the rand-uniform branch August 27, 2018 20:11
@kgryte
Copy link
Contributor Author

kgryte commented Aug 27, 2018

@dsmilkov No problem!

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