Skip to content
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

Add mul gradients #44096

Merged
merged 1 commit into from Oct 26, 2020
Merged

Add mul gradients #44096

merged 1 commit into from Oct 26, 2020

Conversation

vnghia
Copy link
Contributor

@vnghia vnghia commented Oct 16, 2020

@saxenasaurabh

Part of #42668

I would like to ask something:

  • Should we file an issue to track the progress of this project ?
  • Is there any timeline for this project in general ?
  • The test is quite large. Should we seperate it into multiple files ( something like math_grad_test.cc and unified_api_math_test.py )

@google-ml-butler google-ml-butler bot added the size:M CL Change Size: Medium label Oct 16, 2020
@google-cla google-cla bot added the cla: yes label Oct 16, 2020
@gbaned gbaned self-assigned this Oct 16, 2020
@gbaned gbaned added this to Assigned Reviewer in PR Queue via automation Oct 16, 2020
@kkimdev kkimdev requested review from saxenasaurabh and removed request for kkimdev and qqfish October 16, 2020 17:58
@gbaned gbaned added the awaiting review Pull request awaiting review label Oct 19, 2020
@vnghia
Copy link
Contributor Author

vnghia commented Oct 21, 2020

@saxenasaurabh
Could you review this PR please ?

@saxenasaurabh
Copy link
Member

Sorry for the hold up, I will review this today.

@saxenasaurabh
Copy link
Member

Should we file an issue to track the progress of this project ?

That's an interesting idea and something I would definitely like to try. For now I have been tracking issues and progress using Google internal tools. Let me know if you have any suggestions on how we can manage this on github. @dynamicwebpaige may have pointers on managing OSS projects.

Is there any timeline for this project in general ?

I think this project overall will span into Q1'21. In Q4, I want to focus on fleshing out the C++ infrastructure. E.g. Accidental memory leaks are a big problem right now. We also need to make sure that C++ gradient functions are as easy to write as python gradients.

Also getting ResNet training using C++ gradients would be a great milestone! I had done some benchmarking for simple MLPs on MNIST and those were 2x faster in eager mode. If we can get similar gains on ResNet, that would be amazing I think.

I also want to figure out how these gradient functions can be used from python alongside other python gradients. I have this and other things written down in a doc. I will try to publish it externally asap.

The test is quite large. Should we seperate it into multiple files ( something like math_grad_test.cc and unified_api_math_test.py )

That's a great idea. We should try to colocate gradients tests with the gradients code.

PR Queue automation moved this from Assigned Reviewer to Approved by Reviewer Oct 22, 2020
@google-ml-butler google-ml-butler bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Oct 22, 2020
@vnghia
Copy link
Contributor Author

vnghia commented Oct 22, 2020

Let me know if you have any suggestions on how we can manage this on github

I don't know any tool honestly. I am just thinking that we could use a list of checkboxes to maintain the list of gradients but I did not think any further.

@rthadur rthadur added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process and removed awaiting review Pull request awaiting review kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Oct 22, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Oct 22, 2020
@gbaned gbaned added ready to pull PR ready for merge process and removed ready to pull PR ready for merge process labels Oct 26, 2020
@google-ml-butler google-ml-butler bot added the kokoro:force-run Tests on submitted change label Oct 26, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Oct 26, 2020
@copybara-service copybara-service bot merged commit 32249ac into tensorflow:master Oct 26, 2020
PR Queue automation moved this from Approved by Reviewer to Merged Oct 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes ready to pull PR ready for merge process size:M CL Change Size: Medium
Projects
PR Queue
  
Merged
Development

Successfully merging this pull request may close these issues.

None yet

5 participants