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

CleanUp of New GraphToGpu Optimizer #4801

Open
2 of 7 tasks
Sentient07 opened this issue Jul 28, 2016 · 6 comments
Open
2 of 7 tasks

CleanUp of New GraphToGpu Optimizer #4801

Sentient07 opened this issue Jul 28, 2016 · 6 comments
Labels

Comments

@Sentient07
Copy link
Contributor

Sentient07 commented Jul 28, 2016

The cleanup of PR #4570.
Tasks :

  • Correctly handle HostFromGpu nodes ( Replacing them with transfers)
  • Implement backward pass. This can be down with an out2in. This is a new pass after the GraphToGPU.
    • Check if we can op_lifter from the gpuarray_opt EquilibriumOptimizer
    • The backward pass will allow to remove the special case for Alloc / AllocEmpty from GraphToGPU.
  • Check if all the optimizers used by op_lifter is being used by op_lifter_topo
  • Check the abnormal pattern of HostFromGpu and GpuFromHost combination in local_cut_gpu_transfers
  • Factor out caching of Op instances (not needed actually)
@Sentient07
Copy link
Contributor Author

The first four are being done in #4672 . The 5th and 6th could be done with the new register_useless optimizer. The sixth is being done in a different branch which i haven't pushed (Haven't identified how the pattern, HostFromGpu(GpuFromHost(HostFromGpu(Variable)))) is introduced. The seventh is being done in #4728

@nouiz nouiz added the Clean up label Jul 28, 2016
@nouiz
Copy link
Member

nouiz commented Jul 28, 2016

We should not replace op_lifter with out2in. out2in would be a new optimization pass after GraphToGPU. But in the lastest profile, I don't think it will help. But this would help fix the Alloc, AllocEmpty special case.

Mostly out2in is a way to implement the back-ward pass. I'll re-work the description.

@nouiz
Copy link
Member

nouiz commented Jul 28, 2016

I did that. I don't understand this one:

  • Check if all the optimizers used by op_lifter is being used by op_lifter_topo

@nouiz
Copy link
Member

nouiz commented Jul 28, 2016

also, register_useless can't help for the back-ward pass and the op_lifter.
register_useless is before we move to the GPU.

On Thu, Jul 28, 2016 at 11:19 AM Ramana Subramanyam <
notifications@github.com> wrote:

The first four are being done in #4672
#4672 . The 5th and 6th could be
done with the new register_useless optimizer. The sixth is being done in
a different branch which i haven't pushed (Haven't identified how the
pattern, HostFromGpu(GpuFromHost(HostFromGpu(Variable)))) is introduced.
The seventh is being done in #4728
#4728


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#4801 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AALC-9UW3LoOtJ8cBYf3-GeJcM9nVRlLks5qaMiEgaJpZM4JXUm0
.

@Sentient07
Copy link
Contributor Author

Sentient07 commented Jul 28, 2016

We should not replace op_lifter with out2in. out2in would be a new optimization pass after GraphToGPU.

What i meant was replace that from EquilibriumOptimizer as TopoOptimizer with out2_in/

Check if all the optimizers used by op_lifter is being used by op_lifter_topo

By this I meant with the new decorator op_lifter_topo added to all the op_lifters we need to ensure the existing tests pass and there is improvement in profiling results. If there are few optimizers that don't adapt themselves to the op_lifter_topo (like create cycles) we need to fix them.

@Sentient07
Copy link
Contributor Author

also, register_useless can't help for the back-ward pass and the op_lifter.
register_useless is before we move to the GPU.

Yeah, we'd need another optimizer that is like register_useless for the op_lifter optimizers, right ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants