Replace tensor_utils.compile_function() with make_callable

[ryanjulian]

https://www.tensorflow.org/api_docs/python/tf/Session#make_callable

[ahtsan]

There will be two ways of doing it:
Either

def compile_function(inputs, outputs, log_name=None):
    def run(*input_vals):
        sess = tf.get_default_session()
        return sess.make_callable(outputs, feed_list=inputs)(*input_vals)

    return run

f = compile_function(inputs=[obs_var], outputs=[mean_var, log_std_var])
values = f(obs)

or using only tf.Session,

sess = tf.get_default_session()
values = sess.run([mean_var, log_std_var], feed_dict={obs_var: obs})

In my opinion, using only tf.Session is easier to debug since we are clear what tensors are involved when we get the values. However, using compile_function, we can pass the placeholder immediately after building the network, and it will be easier to get the values by directly calling the function.
@ryanjulian What do you think? Should we use make_callable or keep only tf.Session instead?

[naeioi]

@ahtsan The idea of compile_function is to partially bind symbolic input and output of a tf evaluation, so that later you don't need to know the source and sink of a callable. In your first case, there's no point to use sess.make_callable when you can use sess.run. They make no difference.

I think this is the initial intention of this issue:
replace compile_function

def compile_function(inputs, outputs, log_name=None):
    def run(*input_vals):
        sess = tf.get_default_session()
        return sess.run(outputs, feed_dict=dict(list(zip(inputs, input_vals))))

    return run

f = compile_function(inputs=[obs_var], outputs=[mean_var, log_std_var])
values = f(obs)

with make_callable

f = sess.make_callable([mean_var, log_std_var], feed_list=[obs_var])
values = f(obs)

[ahtsan]

but we may not have the tf.Session when we call

f = sess.make_callable([mean_var, log_std_var], feed_list=[obs_var])

so we have to make it into a function, and get the default session later on?

[naeioi]

That makes sense. But if this is the case, then it looks to me that compile_function is perfectly legit. Why bother using make_callable?

[ryanjulian]

I think the idea here is that compile_function makes it opaque which session will call the function, and therefore difficult to debug.

Therefore, using make_callable() (and being forced to decide which session to use at binding time) is a feature, not a bug.

[ryanjulian]

@krzentner your thoughts?

[ahtsan]

Let's assume we will remove compile_function and choose between make_callable() and use naive sess.run.

It will be easy to adopt either way for algorithm since a session is available there anyway, i.e. inside train(). But for policy, make_callable() has to be called when sess is available. Consider the following:

sess = tf.Session()
f = sess.make_callable([policy.outputs], feed_list=[policy.inputs])
...
action = f(obs)
action2 = f(obs2)

and

sess = tf.Session()
...
action = sess.run(policy.outputs, feed_dict={policy.inputs: obs})
action2 = sess.run(policy.outputs, feed_dict={policy.inputs: obs2})

[ryanjulian]

@ahtsan IMO that's a feature, not a bug. compile_function() binds an operation to an unknown session object, which is really confusing and difficult to debug.

magic like that is rarely worth it.

I think you are pointing out that implementing this would take some significant refactoring in the primitives, so that the session is available when a primitive is constructed. I think that would be a positive refactor.

Perhaps this is a large bug to address right now, though, and would be better after models are implemented.

[zhanpenghe]

@ryanjulian hmm I think you miss something from @ahtsan's comments and I agree with him on #326 (comment). Since currently everything is initialized in the __init__ function, while policies are initialized, no session is created and make_callable is not usable yet for them. Of course we can always do a two step initialization for them but I think this will bring us a lot of mess. Currently the sessions are created at the algo.train functions so there is a conflict on the topological order for creating policies' callables and sessions. However I totally agree with replacing compile_function with make_callable after solving this problem.

[ryanjulian]

I think we all agree.

By "significant refactor", I mean that a session already exists when __init__ is called (or when the model is built in some other function).

Let's wait on this one, but keep it in mind as we switch to models.