[ should treat Tensors indices as 1-based #516

[mohamed-180]

Related to #516

x <- as_tensor(1:10)
i <- 1L
x[i] == x[as_tensor(i)] 

gives :
tf.Tensor(True, shape=(), dtype=bool)

[t-kalinowski]

Thank you very much for this PR!
This is actually quite a tricky and big change, and will need to touch the [ codepaths in a few other places.
As it's implemented now, it will work with eager tensors, but will fail if you enter a tf_function() or similar graph tracing context. tfautograph::tf_cond() might be a good way to translate from 1 based to 0 based, or perhaps calling tfautograph::ag_if() directly. Or maybe avoid creating a tf.cond node and instead do some arithmetic on the indexing tensor with max and sign. (It's worth investigating which alternative is better for propagating gradients).

Also, the test suite would need to be expanded so that essentially all the [ tests are evaluated both eagerly and in graph mode.

[mohamed-180]

Can we add tf$executing_eagerly() to the if checks to suppress this behavior on graph mode , to become :

 if (is_tensor(dots) && as.logical(dots > -1L && tf$executing_eagerly())){
       stop_if_any_zeros(list(as.integer(dots)))
       dots <- dots - 1L
     } 

[t-kalinowski]

If I understand correctly, you're suggesting that tensors get passed 'as-is' in a tf_function() but treated as 1-based in eager contexts? I don't think that's a good idea.

The idea of tf_function() is that you can develop your code eagerly, testing individual snippets along the way, and then wrap it in a tf_function() to get a speed boost. If the semantics of things like subsetting change in a tf_function(), that will just lead to nasty surprises for users.

[t-kalinowski]

Closing for now. I wrote some comments about complications and potential approaches here: #519