Migrate from TF_DeprecatedSession to TF_Session

[Minnozz]

This is a prerequisite for supporting SavedModel.

[Minnozz]

With my very limited understanding of TensorFlow, I suspect that the old call to TF_ExtendGraph merges the supplied GraphDef with the current Session, while the new call to TF_GraphImportGraphDef does not consider the existing graph and therefore cannot resolve references to existing nodes.

[Minnozz]

This is probably where the errors are coming from:
https://github.com/tensorflow/tensorflow/blob/fdfc646704c37bdf450525f6ced9d80df86e4993/tensorflow/core/common_runtime/graph_constructor.cc#L726-L736

The "old" ExtendGraph call ended up here:
https://github.com/tensorflow/tensorflow/blob/fdfc646704c37bdf450525f6ced9d80df86e4993/tensorflow/core/common_runtime/graph_execution_state.cc#L181-L286

[Minnozz]

I will try using TF_NewOperation calls instead of using a GraphDef as intermediate.

[fkm3]

That might be a lot of work. If I remember correctly, a lot of the haskell code was designed exploiting the fact that we could build up a graph of protos (without having to interact with the C API or do anything "impure") and then just chuck it all into tensorflow at once via TF_ExtendGraph, so a lot of refactoring might be necessary (I'd be happy to be wrong though).

Not sure if it is feasible, but maybe at the point where TF_ExtendGraph is currently called, instead we could traverse the GraphDef proto in haskell, translating its contents into calls to TF_NewOperation and friends. Of course it seems like a waste to build up the GraphDef and then only use it for that, but it might be the shortest path to a solution.

[Minnozz]

Thanks for the feedback; I'll give it a try to see if it's feasible.

I'm currently using the NodeDef protos in the Builder before they are put into the GraphDef proto, but I think that is equivalent to your suggestion.

[Minnozz]

It is indeed a lot of work. I will try using the TF_GraphImportGraphDef approach again, but this time with an input mapping for existing nodes in the graph.

[Minnozz]

Passing an input map seems to work.

[Minnozz]

@fkm3 Thanks for looking into this PR; can you tell something about the CI failure?

[fkm3]

It is just some warnings-as-errors, you should be able to reproduce using the --pedantic flag of stack or ghc (or use the CI docker script).

tensorflow                 > [ 1 of 12] Compiling TensorFlow.Internal.Raw
tensorflow                 > 
tensorflow                 > /[tfhs/tensorflow/src/TensorFlow/Internal/Raw.chs:86](https://cs.corp.google.com/piper///depot/google3/tfhs/tensorflow/src/TensorFlow/Internal/Raw.chs?l=86):29: error: [-Wname-shadowing, -Werror=name-shadowing]
tensorflow                 >     This binding for ‘ptr’ shadows the existing binding
tensorflow                 >       bound at [src/TensorFlow/Internal/Raw.chs:86](https://cs.corp.google.com/piper///depot/google3/src/TensorFlow/Internal/Raw.chs?l=86):10
tensorflow                 >    |
tensorflow                 > 86 |     peek ptr = Output <$> {# get TF_Output->oper #} ptr
tensorflow                 >    |                             ^^^
tensorflow                 > 
tensorflow                 > /[tfhs/tensorflow/src/TensorFlow/Internal/Raw.chs:87](https://cs.corp.google.com/piper///depot/google3/tfhs/tensorflow/src/TensorFlow/Internal/Raw.chs?l=87):47: error: [-Wname-shadowing, -Werror=name-shadowing]
tensorflow                 >     This binding for ‘ptr’ shadows the existing binding
tensorflow                 >       bound at [src/TensorFlow/Internal/Raw.chs:86](https://cs.corp.google.com/piper///depot/google3/src/TensorFlow/Internal/Raw.chs?l=86):10
tensorflow                 >    |
tensorflow                 > 87 |                       <*> (fromIntegral <$> {# get TF_Output->index #} ptr)
tensorflow                 >    |                                               ^^^
tensorflow                 > 
tensorflow                 > /[tfhs/tensorflow/src/TensorFlow/Internal/Raw.chs:89](https://cs.corp.google.com/piper///depot/google3/tfhs/tensorflow/src/TensorFlow/Internal/Raw.chs?l=89):11: error: [-Wname-shadowing, -Werror=name-shadowing]
tensorflow                 >     This binding for ‘ptr’ shadows the existing binding
tensorflow                 >       bound at [src/TensorFlow/Internal/Raw.chs:88](https://cs.corp.google.com/piper///depot/google3/src/TensorFlow/Internal/Raw.chs?l=88):10
tensorflow                 >    |
tensorflow                 > 89 |         {# set TF_Output->oper #} ptr oper
tensorflow                 >    |           ^^^
tensorflow                 > 
tensorflow                 > /[tfhs/tensorflow/src/TensorFlow/Internal/Raw.chs:90](https://cs.corp.google.com/piper///depot/google3/tfhs/tensorflow/src/TensorFlow/Internal/Raw.chs?l=90):11: error: [-Wname-shadowing, -Werror=name-shadowing]
tensorflow                 >     This binding for ‘ptr’ shadows the existing binding
tensorflow                 >       bound at [src/TensorFlow/Internal/Raw.chs:88](https://cs.corp.google.com/piper///depot/google3/src/TensorFlow/Internal/Raw.chs?l=88):10
tensorflow                 >    |
tensorflow                 > 90 |         {# set TF_Output->index #} ptr $ fromIntegral index
tensorflow                 >    |           ^^^

[Minnozz]

Oops, thought I checked that. Fixed now.

[fkm3]

Thanks for figuring this out. I think a handful of people including myself have considered doing this and bounced off it.

[Minnozz]

@fkm3 I think I've addressed your feedback, what do you think of the current PR?

[fkm3]

looks good!