eaplatanios / tensorflow_scala

TensorFlow API for the Scala Programming Language
http://platanios.org/tensorflow_scala/
Apache License 2.0
936 stars 96 forks source link

Remove overly restrictive assertion in addNOutputIndexedSlices #50

Closed carlo-veezoo closed 6 years ago

carlo-veezoo commented 6 years ago

The assertion in addNOutputIndexedSlices is overly restrictive, because the denseShape might be always equal during execution despite the associated Output being different. This breaks the following code:

import org.platanios.tensorflow.api._
import org.platanios.tensorflow.api.ops.variables.ZerosInitializer

object GradientAggregation {
  def main(args: Array[String]): Unit = {
    val a = tf.variable("a", FLOAT32, Shape(1), ZerosInitializer)
    val concatenated = tf.concatenate(Seq(a, a))
    val gathered = tf.gather(concatenated, Tensor(1))
    val loss = gathered.sum()
    tf.Gradients.gradients(Seq(loss), Seq(a))
  }
}

I hesitated to add a tf.assert to dynamically check the shape, but I don't think you would always want that because of the possible slowdown.

eaplatanios commented 6 years ago

You’re right. What I was doing there doesn’t make any sense actually. I think we should avoid the dynamic assert due to performance reasons, as you also mention. It’s safe to assume this will always hold given that it’s always aggregating gradients generated for the same arguments. Let’s just leave it with no assertion for now. :)