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

Add tf.normalize to provide a clean interface for tensor normalization #28741

Closed
sleighsoft opened this issue May 15, 2019 · 9 comments
Closed
Assignees
Labels
comp:ops OPs related issues stat:contribution welcome Status - Contributions welcome TF 2.0 Issues relating to TensorFlow 2.0 type:feature Feature requests

Comments

@sleighsoft
Copy link
Contributor

System information

  • TensorFlow version (you are using): 1.3+ or 2.0a
  • Are you willing to contribute it (Yes/No): Yes

Describe the feature and the current behavior/state.
Currently there are two "ways" to normalize vectors, matrices, and tensors.

  1. tf.math.l2_normalize
  2. tf.norm

The first one directly normalizes the input tensor and returns it.
The latter only computes the norm and the user has to do the division to obtain a normalized tensor.

I propose to add a function called tf.math.normalize which has the same/similar args as tf.norm. It computes the normalized tensor as tf.math.l2_normalize does but supports all the normalizations that tf.norm supports.

Will this change the current api? How?
No, it just adds a cleaner interface to do normalization.
In the long run tf.math.l2_normalize would be obsolete, though. It's implementation could be removed and it could use tf.normalize internally.

Who will benefit with this feature?
When looking for a way to perform l1 normalization I only found l2_normalize and tf.norm. I then expected there to exist a l1_normalize function as well which is not the case.
From my point of view this is an inconsistency that people will encounter from time to time.

I would propose to only add this to tensorflow 2.0 and maybe also remove tf.math.l2_normalize in that case.

@gadagashwini-zz gadagashwini-zz self-assigned this May 16, 2019
@gadagashwini-zz gadagashwini-zz added type:feature Feature requests 2.0.0-alpha0 comp:ops OPs related issues labels May 16, 2019
@ymodak ymodak assigned alextp and unassigned ymodak May 17, 2019
@ymodak ymodak added the stat:awaiting tensorflower Status - Awaiting response from tensorflower label May 17, 2019
@alextp alextp added stat:contribution welcome Status - Contributions welcome and removed stat:awaiting tensorflower Status - Awaiting response from tensorflower labels May 17, 2019
@alextp
Copy link
Contributor

alextp commented May 17, 2019

Sounds reasonable. I don't think we can deprecate the existing endpoint but I'm happy to add a new one, if someone wants to contribute.

@astropeak
Copy link
Contributor

astropeak commented May 19, 2019

Hey, I'd like to take a crack at this! Do we add this function as tf.normalize, or tf.math.normalize?

@sleighsoft
Copy link
Contributor Author

@astropeak, I am also happy to contribute, maybe we can work together on this?

@astropeak
Copy link
Contributor

@sleighsoft , in that case, maybe you could just do it. I haven't started and could always take other tasks.

@astropeak
Copy link
Contributor

But if the cooperation is a better way, I am happy to work together. How to split the tasks? How we commit codes?
Anyway, I am totally OK that only you work on the task if the cooperation isn't a good choice.

@sleighsoft
Copy link
Contributor Author

sleighsoft commented May 21, 2019

@astropeak It looks like you already have some other parts of tensorflow in your pipeline. I would like to give it a try and would then like to get your advice once I open a PR. If you already have ideas and suggestions feel free to add them here.
My intial thought was to do it along the lines of sklearn.preprocessing.normalize and numpy.linalg.norm.

@kmh4321
Copy link
Contributor

kmh4321 commented May 29, 2019

@sleighsoft @astropeak is someone still working on this?

@sleighsoft
Copy link
Contributor Author

I am, just don't have much spare time right now to make fast progress.

@jvishnuvardhan
Copy link
Contributor

@sleighsoft I see your PR merged. If this issue was resolved by your PR, Please close. Thanks!

@jvishnuvardhan jvishnuvardhan added TF 2.0 Issues relating to TensorFlow 2.0 and removed TF 2.0.0-alpha0 labels Sep 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:ops OPs related issues stat:contribution welcome Status - Contributions welcome TF 2.0 Issues relating to TensorFlow 2.0 type:feature Feature requests
Projects
None yet
Development

No branches or pull requests

7 participants