Recursively copying elements from one graph to another

[srjoglekar246]

Allows for easy portability of elements (Variables and Ops) from one Tensorflow Graph to another. If called on a top-level root in a dataflow graph, automatically copies all required instances.

Provides an API to retrieve the copied elements in the other graph, using a namespace.

Reference: https://codesachin.wordpress.com/2015/11/20/recursively-copying-elements-from-one-graph-to-another-in-tensorflow/

[srjoglekar246]

Just added the code, haven't changed any imports yet. Want to get general feedback before refining the code.

[vrv]

Hi @srjoglekar246, this looks pretty cool, and thanks for going through the work to put this together!

One of the ideas we have been working on is a notion of "Functions" in the GraphDef, which would allow for re-usable components. See here for the proto definition.

It's not ready yet and still needs some work (we may not do this as a proto in the GraphDef) but the underlying idea would be something that would obviate the need for copying elements around with unique names: you could define a subgraph as a function with inputs and outputs, and then re-use them. So instead of having multiple versions of the graph stamped out at once, each with unique names, you could have one named function that could be called, ported across graphs, etc.

Do you feel that such a feature would accomplish your higher level goals?

[srjoglekar246]

Aah yes. The whole idea was to enable reusability of dataflow-structures across Graph instances. If I am not wrong, your solution would need some careful handling of how sessions 'run' these portable functions. But if implemented right, it could save a lot of memory and accomplish what my code intends.

[vrv]

Yeah, there's some more plumbing in the internal execution of graphs that specially handles functions.

For now, do you mind if we keep this pull request open but on the backburner, at least until we figure out whether functions may be an easier to use abstraction?

(If you really want this checked in somewhere and know lots of others are using this, we've been intending to create a 'contrib' directory or repo where these types of utilities / functions could be placed).

[srjoglekar246]

Yeah no problem!
On the other hand, a contrib directory for such scripts would be nice - especially for code that might be useful to a good audience as utilities (instead of being inside the main framework).

[tensorflow-jenkins]

Can one of the admins verify this patch?

[srjoglekar246]

Ping @vrv

[vrv]

We've considered adding a contrib directory but ownership and bug reports would be hard to manage -- probably needs to be a separate repo. Adding @martinwicke since I think he's in the process of figuring this out.

There's been more progress over the past few weeks on functions. I think it's close -- take a look at an example: https://github.com/tensorflow/tensorflow/blob/master/tensorflow/python/framework/function_test.py#L279 and let us know if that would sort of accomplish what you want

[bhack]

@vrv With the experience of opecv-contrib it is hard to maintain a quality level in a contributed repository and grant a decent review time of that PRs. One of the best community scaling effort is the Debian developer/maintainer process. We can implement a very light version of this here. Github lowered the entrance cost of new developers trough the fork-PR process but also let proliferation of sparse or short time frame contributions. We can find a way to reward middle and long term valid contributors or contributors groups to maintain some contrib modules in Tensorflow and to review and accept related PR and issue on this target modules. I don't know what will be the best way to to this with the actual Github management features for handling process of module Orphaning, contributor/group MIA, new module proposal acceptance and module removal. I think that Wikifing community rules a little bit, using labels, repositories, and submodules the process could be managed in some way.

[srjoglekar246]

@vrv A separate repo would be nice, would even let users build a proper codebase for different algorithms implemented in tensorflow. Something like what sklearn is for scipy.
As far as the code goes, it pretty much does what I wanted to achieve, with a better interface for reusable functions across graphs. Is it coming out in the next version? The function class would also enable running of different algorithms within same environment - a nice bonus.

[vrv]

@srjoglekar246, @bhack: we'll try to find something that works for contributions.

As for functions: I'm not entirely sure what the state of it is, but I wanted to solicit early feedback from you since it seems like what you originally were trying to do with this PR. It's being actively worked on so I'm hoping it will be ready "soon".

[srjoglekar246]

I guess once we can define reusable functions, we could do away with initialising ops as "tf.add", instead going for the method used here: https://github.com/tensorflow/tensorflow/blob/master/tensorflow/python/framework/function_test.py#L99 . My only suggestion would be to make the system smarter with respect to argument types (like adding float types to int types should automatically return a float, assuming shapes are compatible). But I guess it won't be too easy, especially in C++.

[cesarsalgado]

I'm afraid that creating a separate repo would lose the focus of the community as a whole.

Edit: For example, I would like all implementations of new papers to have high quality documentation. I'm afraid that the the contrib repo would have a poor doc like some caffe PR has. Maybe if tensorflow has semi-offical implms of some paper this will disincentivize the main repo to make an official and better documented implem earlier than it would otherwise.

[martinwicke]

Sorry for the long silence -- if you're still interested, I'd like to merge this into contrib. Can you move the file to tensorflow/contrib/copy_graph/python/util/copy.py?

Also, can you add a license header and the python3 from future imports, and can you add a test for this?

[tensorflow-jenkins]

Can one of the admins verify this patch?

[srjoglekar246]

@martinwicke Will get it done by tomorrow. Any particular format/guideline for the unit test to be added?

[martinwicke]

Just make sure that it tests the functionality you claim your functions provide.

[martinwicke]

And can you modify the docstrings to match the tensorflow style guide (look at the "writing documentation" howto)?

Thanks!

[srjoglekar246]

@martinwicke Could you take a look at the code (especially the BUILD and test files) and tell me if I am on the right track?

[martinwicke]

Can you import this before tensorflow

[martinwicke]

Can you add this module to gen_docs_combined.py? See the other contrib modules in there.

[srjoglekar246]

Sure. Can you let me know if/how to run the tensorflow tests on my machine?

[martinwicke]

Follow the instructions to build from source. If you can do that, you should be able to do
bazel test tensorflow/... to run the tests. You can also give explicit test targets to re-run only some tests.

[srjoglekar246]

@martinwicke Made the changes. Let me know if there's anything more to be modified.

[srjoglekar246]

Ping @martinwicke

[gunan]

Can one of the admins verify this patch?

[martinwicke]

Jenkins, test this please.

[martinwicke]

This import seems redundant here.

[martinwicke]

I've had some minor comments about python module things, but otherwise looks good.

[srjoglekar246]

@martinwicke I made the changes, and fixed the bazel test errors. They all pass now. Have a look.

[martinwicke]

Thanks!

Jenkins, test this please.

[srjoglekar246]

@martinwicke Seems to work. Okay to be pushed in?

[martinwicke]

Thanks!

[thjashin]

Hi @vrv ,

I'm currently writing some high-level library based on tensorflow. I'm relying a lot on copying existing ops to achieve re-usability of data flow structures. So I'm very interested in the "Functions" idea you mentioned here. I'm wondering how this is going on.
Because I recently met some problems due to not copying op._control_flow_context, which makes gradients through tf.cond fail in copied subgraph. This problem also exists in this contribution.

[vrv]

There's been some work on functions but it's still kind of primitive and I don't know how well it composes with control flow.

We have https://github.com/tensorflow/tensorflow/blob/5a566a7701381a5cf7f70fce397759483764e482/tensorflow/python/framework/function.py which isn't yet public (but until we seal the public interface it's still available to play around with), and it isn't getting that much love / attention unfortunately. But if it proves useful, let us know and maybe we can at least make it public at some point.

[thjashin]

@vrv Thanks for the link. I had a look at functions and unfortunately that's not what I actually want. I can describe my high-level goals here. It's something like theano.clone() (related issues: #5479 #1070), which, in my view, can be seen as operation-level reuse rather than subgraph-level, which enables one to replace inputs of any operations in the graph. I guess this is also what the author of tf.contrib.graph_editor tries to achieve.