Skip to content

Conversation

@charmasaur
Copy link
Contributor

@charmasaur charmasaur commented Feb 24, 2021

This adds gradient support for complex, and fixes a bug with adjoints (#47337).

My laptop can't build, so I haven't been able to properly test this yet. Hopefully CI will do it?

@google-ml-butler google-ml-butler bot added the size:M CL Change Size: Medium label Feb 24, 2021
@google-cla google-cla bot added the cla: yes label Feb 24, 2021
@charmasaur charmasaur force-pushed the complex_sparse_grad branch 2 times, most recently from 9ac90fd to 2ee44de Compare February 24, 2021 00:55
@charmasaur charmasaur marked this pull request as ready for review February 24, 2021 01:00
@gbaned gbaned self-assigned this Feb 24, 2021
@gbaned gbaned added the comp:ops OPs related issues label Feb 24, 2021
@gbaned gbaned requested a review from allenlavoie February 24, 2021 03:40
For a start the test wasn't being run. But it also had a problem where
the test in the complex case was trivial (the matrices) were all zeros:
it was creating `x` in `[0,1]`, then turning it into `(x+1j*x)/2` via
`_maybe_complex`, and then setting all the values with magnitude below
`0.8` to `0`. But even `(1+1j)/2` (the value with largest magntidue that
`x` can take) only has a magnitude of about `0.7`. Fix is just to create
complex numbers as `x+x*1j` instead.
@charmasaur
Copy link
Contributor Author

I'm having trouble testing this locally because I can't build TF (I'm just on a little laptop :C), but CI seems to have run everything successfully at 0623c04. I don't think the test update I've made since then should be a problem, but hard to know until it runs.

Copy link
Member

@allenlavoie allenlavoie left a comment

Choose a reason for hiding this comment

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

Looks great, nice fix!

@google-ml-butler google-ml-butler bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Feb 24, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Feb 24, 2021
@charmasaur
Copy link
Contributor Author

Thanks! And thanks for the review :)

@gbaned gbaned added ready to pull PR ready for merge process and removed ready to pull PR ready for merge process labels Feb 25, 2021
@copybara-service copybara-service bot merged commit ddeb3ab into tensorflow:master Feb 25, 2021
@charmasaur charmasaur deleted the complex_sparse_grad branch February 25, 2021 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes comp:ops OPs related issues ready to pull PR ready for merge process size:M CL Change Size: Medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants