Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added batch_gather to backend. #6377

Closed
wants to merge 1 commit into from

Conversation

phipleg
Copy link
Contributor

@phipleg phipleg commented Apr 23, 2017

Adds the method batched_gather(reference, indices which is a batched variant of gather where indices is a 1d tensor.

The numpy equivalent is reference[np.arange(batch_size), indices]. For example, if reference is [[3, 5, 7], [11, 13, 17]] and indices is [2, 1] then the result is [7, 13].

While the numpy syntax can be be directly translated to Theano, due to slicing restrictions this approach does not work in Tensorflow. Instead one can use tf.gather_nd but there is no direct support in Keras.

This extension would allow to implement the backward forward algorithm for the inference step of a Conditional Random Field with Keras (see PR #4621). It is the last necessary modification of the backend for implementing a CRF in Keras.

@fchollet
Copy link
Collaborator

This function is not present in either Theano or TensorFlow. This makes it a harder sell.

This extension would allow to implement the backward forward algorithm for the inference step of a Conditional Random Field with Keras

If this is the rationale, then we should package such an addition with a CRF layer (at a later time), rather than including the function in core Keras right now. We would rather not have backend functions that:

  • are not used by anything in the Keras codebase
  • are not a direct mirroring of the TF API

@phipleg
Copy link
Contributor Author

phipleg commented Apr 25, 2017

Thank you very much for your answer, @fchollet. I completely agree and will try to find another solution for the batch_gather implementation without touching the backend.

@fchollet fchollet closed this Apr 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants