search

eaplatanios / tensorflow_scala

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

Implement AddAggregationMethod for OutputIndexedSlices #48

Closed carlo-veezoo closed 6 years ago

carlo-veezoo commented 6 years ago

Sorry to bombard you with pull requests, if you prefer another channel for these small improvements, please tell me :) Anyways, I've implemented AddAggregationMethod for OutputIndexedSlices, the implementation is correct, I hope. Some notes: Maybe you want to add a check to make sure that all inputs have the same dense shape (not sure how to do that), or refactor so that addNOutputIndexedSlices is a public function somewhere.

eaplatanios commented 6 years ago

No that is definitely a great way of submitting those changes. Thanks a lot for actually giving my library and shot and also spending time to implement missing features that you need. That's great! :)

Regarding the check on the shape. If all is right we should not be needing that because the indexed slices being added represent gradients of the same tensors and with respect to the same tensor. So, they should have the same shape. If they do not there must be a bug somewhere else. I may add an assertion though just to make sure. :)

carlo-veezoo commented 6 years ago

@eaplatanios Cool, thanks a lot!