tensorflow / addons

Useful extra functionality for TensorFlow 2.x maintained by SIG-addons
Apache License 2.0
1.69k stars 611 forks source link

Adding a pure python implementation corresponding to simple C++/Cuda ops. #1114

Closed gabrieldemarmiesse closed 1 year ago

gabrieldemarmiesse commented 4 years ago

Describe the feature and the current behavior/state.

For custom ops, we have two implementations: C++ and Cuda. I propose that for simple ops , meaning less than 10 python lines, we implement a pure Python version too. I see several use cases:

Relevant information

Which API type would this fall under (layer, metric, optimizer, etc.)

Activation, image, layers, seq2seq, text

Who will benefit with this feature?

People having problems with custop ops, the maintainers because the test suite will be more robust.

Any other info.

We should make a pull request for a simple op see how to implement it. Especially how do we access the different implementations from the user side and tests side. Good candidates would be activations functions.

failure-to-thrive commented 4 years ago

Frankly, it's not clear for me why TFA activations/... need C++ code. All those functions can be expressed with regular Tensorflow ops, gaining the same benefits such as a GPU placement. What did I miss?

gabrieldemarmiesse commented 4 years ago

https://github.com/tensorflow/addons/pull/913 can give some background and discussion that we had concerning this.

seanpmorgan commented 4 years ago

I really like the idea of this and it's ability to test python vs custom-op implementations is much needed. I'm a little squeemish regarding the flag to do this though. Do you imagine it would be a flag in for example the activation API or some global setting?

gabrieldemarmiesse commented 4 years ago

I've no idea what kind of flag we should use for the public API. Ideas welcome :)

georgesterpu commented 4 years ago

@gabrieldemarmiesse what do you think of porting the Beam search decoder from tensorflow / models / official / nlp / transformer / beam_search.py to addons as a python alternative ?

failure-to-thrive commented 4 years ago

Also please keep mind that that you should add python implementations of gradients, as C++ do. Autodeduction of them is quite suboptimal and should be hand-crafted.

gabrieldemarmiesse commented 4 years ago

@georgesterpu it seems a bit too complex. I'm afraid it would actually become a maintainance burden instead of helping us. It might be better to not copy the implementation here.

gabrieldemarmiesse commented 4 years ago

I made pull requests to expose the implementations publicly:

1137

1250

1251

1252

1253

bhack commented 4 years ago

@georgesterpu I dont know if it could be better that you propose the adoption in https://github.com/tensorflow/text. So if they will get the ops in charge we could evaluate to deprecate, soon or later, the custom c++/cuda version in addons.

guillaumekln commented 4 years ago

For reference, #1925 is adding a Python implementation for tfa.seq2seq.gather_tree which should simplify model export.

seanpmorgan commented 1 year ago

TensorFlow Addons is transitioning to a minimal maintenance and release mode. New features will not be added to this repository. For more information, please see our public messaging on this decision: TensorFlow Addons Wind Down

Please consider sending feature requests / contributions to other repositories in the TF community with a similar charters to TFA: Keras Keras-CV Keras-NLP