search

tensorflow / addons

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

Re-name all C++ Ops to include package name #379

Closed seanpmorgan closed 5 years ago

seanpmorgan commented 5 years ago

Per new RFC: https://github.com/tensorflow/community/pull/126

seanpmorgan commented 5 years ago

Just copying a note I made in the RFC: This is currently blocked as > is an invalid character in an Op Name at the moment

WindQAQ commented 5 years ago

A minor thing related to package name is the ifndef/define in header files. We have to unify this too.

https://github.com/tensorflow/addons/blob/master/tensorflow_addons/custom_ops/image/cc/kernels/adjust_hsv_in_yiq_op.h#L14 https://github.com/tensorflow/addons/blob/master/tensorflow_addons/custom_ops/image/cc/kernels/image_projective_transform_op.h#L16 https://github.com/tensorflow/addons/blob/master/tensorflow_addons/custom_ops/layers/cc/kernels/correlation_cost_op.h#L16 https://github.com/tensorflow/addons/blob/master/tensorflow_addons/custom_ops/seq2seq/cc/kernels/beam_search_ops.h#L16

karmel commented 5 years ago

I believe this should be unblocked from the TF side now: https://github.com/tensorflow/tensorflow/commit/3bda03617967d3ff3c9af0e96c8a66b2bdad6ffc

karmel commented 5 years ago

(CC @ewilderj , FYI)

tomerk commented 5 years ago

I'm putting together a pr to do this.

tomerk commented 5 years ago

I just opened the PR. I had to make some changes to TF as part of this because there were still bugs in the TF RC1 w/ namespaces in op names, so this does not work w/ RC1. It works w/ the TF 2.0 nightly & should work w/ the upcoming RC2.

facaiy commented 5 years ago

Closed as it's fixed by #516