-
Notifications
You must be signed in to change notification settings - Fork 561
Set three_fry as the default RNG for GPU #3951
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
Conversation
JackCaoG
left a comment
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.
There were some ci error before, not sure if it is related.
|
I'm able to reproduce the CI error locally. It looks like changing RNG happened to generate invalid inputs for |
|
Hmm, can you give me a bit more context, I assume that there is a input which |
|
It looks like this issue is not specific to three_fry RNG. Below is the minimum code to reproduce: import numpy as np
import torch
import torch_xla.core.xla_model as xm
import os
os.environ['GPU_NUM_DEVICES'] = '1'
os.environ['XLA_RNG_BIT_GENERATOR'] = 'default'
device = xm.xla_device()
count = 1
x = torch.testing.make_tensor((2,1),dtype=torch.float, device=device)
y = torch.randint(1, 3, (1,), device=device)
z = torch.testing.make_tensor((1,), dtype=torch.float, device=device, low=1)
x = torch.testing.make_tensor((2,1),dtype=torch.float, device=device)
y = torch.tensor([count], dtype=torch.int64, device=device)
z = torch.testing.make_tensor((1,), dtype=torch.float, device=device, low=1)
xm.mark_step()
w = torch.cov(x,correction=2,fweights=y,aweights=z)
xm.mark_step()
print(w)
print(np.cov(x.cpu().numpy(), ddof=2, fweights=y.cpu().numpy(), aweights=z.cpu().numpy()))With outputs: If I change I checked the pytorch implementation and it looks So my guess is that the discrepancy is due to I've disabled this test since this op is not officially supported by pytorch/xla (due to lack of torch.equal support) and the error appears to be irrelevant. |
JackCaoG
left a comment
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.
Thanks!
As discussed in #3868, three_fry demonstrates better performance than the default (philox) RNG on GPU. So we should consider making three_fry the default RNG on GPU.
Below are some models I've tested:
cc @JackCaoG @ang868