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

Feature: Ability to return normalised vectors #517

Open
wants to merge 8 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@DomHudson
Contributor

DomHudson commented May 8, 2018

Summary

L2 norms can be very useful. This PR adds the ability to normalise vectors with the L2 norm. Can be called via the python interface with

get_word_vector(self, word, normalise = False)
@facebook-github-bot

This comment has been minimized.

facebook-github-bot commented May 8, 2018

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

@facebook-github-bot

This comment has been minimized.

facebook-github-bot commented May 8, 2018

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@EdouardGrave

Thank you for your pull request!

This looks almost good to me: please make the small changes before we can merge.

Best,
Edouard

@@ -69,6 +69,10 @@ void FastText::getWordVector(Vector& vec, const std::string& word) const {
if (ngrams.size() > 0) {
vec.mul(1.0 / ngrams.size());
}
if(normalise) {
vec.normalise();

This comment has been minimized.

@EdouardGrave

EdouardGrave Jun 5, 2018

Member

Could you please use

real norm = vec.norm();
if (norm > 0.0) {
  vec.mul(1.0 / norm);
}

instead? And remove the changes in vector.cc and vector.h. Thanks.

@DomHudson

This comment has been minimized.

Contributor

DomHudson commented Jun 6, 2018

Hi Edouard,

Thank you for the review. I have updated the code with your changes.

Many thanks,
Dom

@facebook-github-bot

@EdouardGrave is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@DomHudson

This comment has been minimized.

Contributor

DomHudson commented Nov 7, 2018

Any update on this? Would be great if we could merge in

Many thanks
Dom

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment