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
Rand function for complex dtype #34924
Conversation
Hi @anjali411 can you take look at this? I can't edit reviewers, also not sure why ci stopped at only 6 checks. Side note didn't run into issue with |
💊 CircleCI build failures summary and remediationsAs of commit c8e8e74 (more details on the Dr. CI page):
🕵️ 2 new failures recognized by patternsThe following build failures do not appear to be due to upstream breakages (reran 1 job to discount flakiness): pytorch_xla_linux_xenial_py3_6_clang7_build (1/2)Step: "Build" (full log | pattern match details) <confirmed not flaky by 2 failures>
|
Sure @anjali411 I will do that. Correct me if I am wrong but doesn't |
oh yeah you are right! randn calls into uniform_ which has separate cpu and cuda calls so we don't need to worry about it. This PR looks good overall. |
@anjali411 updated |
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.
@anjali411 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
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.
LGTM :) thanks for the awesome work!
if (result.is_complex()) { | ||
auto float_tensor = at::native::view_complex_as_float(result); | ||
float_tensor.uniform_(0, 1, generator); | ||
// we want to return complex tensor not underlaying float tensor. |
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.
nit: typo- not underlying
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.
oops, fixed
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.
@anjali411 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
@kostekIV two tests seem to be failing due to this PR. can you look into it? |
@anjali411 I am honestly at lost here, I was looking for an error and couldn't find what is causing those failures here. I looked into other prs and few of the already merged ones had this 2 failures, for example this had |
@anjali411 merged this pull request in ada4077. |
Address #34380