Clean up

[Sentient07]

The cleanup of PR #4570. Will rebase with the master once #4570 gets merged.
Tasks :

• Correctly handle HostFromGpu nodes
• Remove the special case for Alloc / AllocEmpty from the new optimizer, and register these lifters in an "out2in" topooptimizer
• Replace op_lifter with out2in
• Factor out caching of Op instances
• Check if all the optimizers used by op_lifter is being used by op_lifter_topo
• Implement backward pass.
• Remove op_lifter from EquilibriumOptimizer

@lamblin Am i missing anything in the to-do list?

[Sentient07]

I felt it is better to pass the context_name. Will change if necessary

[lamblin]

As documented here, the best practice is to infer the context during make_node, not to force the context when creating the Op.
Please remove the context_name attribute.

[Sentient07]

Yeah, i'll remove that commit.

[Sentient07]

In line:467 of theano/gpuarray/type.py, if i make a call to transfer instead of host_from_gpu, it gives ,
RuntimeError: maximum recursion depth exceeded

[Sentient07]

In this PR, i've removed ShapeOptimizer from fast_compile. There are some optimizations that need ShapeOptimizer and is a part of fast_compile. I'm removing those optimizations from fast_compile. Yet to open that PR of removing ShapeOptimizer. @lamblin I believe that PR should get merged before this ?

[lamblin]

Use something between -0.5 and 1 instead of 10 here.

[lamblin]

It seems to make sense for a first draft of the backward pass (using TopoOptimizer and LocalGroupDB).
When you change the priorities so it is executed before the EquilibriumOptimizer, we can see in the profiling if there is something to gain, and if we need to optimize the implementation further.
For instance, maybe we would need to implement tracks in LocalGroupDB, like in EquilibriumDB.

[Sentient07]

For instance, maybe we would need to implement tracks in LocalGroupDB, like in EquilibriumDB.

Could you please elaborate on this ?

[lamblin]

EquilibriumDB has "tracks", or ops to match when trying to apply its local optimizers. For instance, some local optimizers will only be tried on nodes corresponding to some Ops. It uses the Ops passed to the local_optimizer decorator.
For instance, if a local opt registered with @local_optimizer([GpuFromHost, GpuToGpu, HostFromGpu]), then the EquilibriumOptimizer will only try to apply it on nodes where node.owner.op is an instance of those types.
I'm not sure if the LocalGroup uses this information. If it does not, then we may need to implement that logic, like in EquilibriumOptimizer.

[Sentient07]

From what I see,LocalGroup does not use that info. LocalOptimizer uses track but I don't think LocalGroup does. I'll confirm again once.

[Sentient07]

@nouiz The new optimizer op_lifter_topo doesn't show all the local optimizers as gpu_optimizer. Am I doing the query wrong ?

[theano-bot]

Can one of the admins verify this patch?

[Sentient07]

We're registering theano.tensor.opt.local_remove_all_assert into gpu_optimizer separately. Don't we need to do the same for gpu_optimizer2 ?

[nouiz]

Why this change? Just remove this commit, or explain why it is needed.

[Sentient07]

oh! This should have gotten removed from the rebase. It existed before, but i have changed this behaviour in #4570 itself

[Sentient07]

Yeah, this diff doesn't exist anymore. It has been removed. Earlier, i thought it was better to pass context_name

[Sentient07]

There are few redundant commits. #128f257 and #3855671 (Did in #4570 but with a different name), #5bbbb31 did as a different PR. Shall I squash them ?

[lamblin]

We will need to the the automatic cache (in Op's metaclass) first, and then go on with the cleaning up.
We will not be able to do that for 0.9, so bumping to 0.10

[Sentient07]

Closing as continued in #5579